From b0f22f7ed4ece7d2064ade7c829e82964b9f7219 Mon Sep 17 00:00:00 2001 From: Robo Date: Wed, 6 Nov 2024 01:22:46 +0900 Subject: [PATCH] chore: backport editcontext ime input alignment fixes (#44572) Refs https://issues.chromium.org/issues/351029417 --- patches/chromium/.patches | 2 + ...xels_from_firstrectforcharacterrange.patch | 138 ++++++++++++++++++ ...ol_and_selection_bounds_as_fallbacks.patch | 83 +++++++++++ ...available_to_oom_exception_arguments.patch | 3 +- 4 files changed, 224 insertions(+), 2 deletions(-) create mode 100644 patches/chromium/editcontext_return_physical_pixels_from_firstrectforcharacterrange.patch create mode 100644 patches/chromium/editcontext_use_empty_control_and_selection_bounds_as_fallbacks.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index e7fdc1f822..355ab5f7e0 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -136,3 +136,5 @@ feat_allow_-4_as_a_macos_screen_share_id.patch fix_software_compositing_infinite_loop.patch cherry-pick-8c4edae5e34d.patch oom_add_commit_limit_available_to_oom_exception_arguments.patch +editcontext_use_empty_control_and_selection_bounds_as_fallbacks.patch +editcontext_return_physical_pixels_from_firstrectforcharacterrange.patch diff --git a/patches/chromium/editcontext_return_physical_pixels_from_firstrectforcharacterrange.patch b/patches/chromium/editcontext_return_physical_pixels_from_firstrectforcharacterrange.patch new file mode 100644 index 0000000000..cc3bf36f3c --- /dev/null +++ b/patches/chromium/editcontext_return_physical_pixels_from_firstrectforcharacterrange.patch @@ -0,0 +1,138 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Daniel Clark +Date: Mon, 5 Aug 2024 21:08:17 +0000 +Subject: Return physical pixels from FirstRectForCharacterRange + +EditContext::FirstRectForCharacterRange returns its result in CSS +pixels, but the caller expects to receive a result in physical pixels, +resulting in incorrect IME positions on high-DPI displays. + +Switch the function to return its result in physical pixels. + +Bug: 351029417 +Change-Id: I846b2a62856804274b1d106028efbd5489181f80 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5749469 +Commit-Queue: Dan Clark +Reviewed-by: Koji Ishii +Cr-Commit-Position: refs/heads/main@{#1337483} + +diff --git a/third_party/blink/renderer/core/editing/ime/edit_context.cc b/third_party/blink/renderer/core/editing/ime/edit_context.cc +index 6c7462ef353f23c8fdfdd9b874f08e506a5c27cd..b41b6c4a51dffde71df57a0dc34b325733c8f342 100644 +--- a/third_party/blink/renderer/core/editing/ime/edit_context.cc ++++ b/third_party/blink/renderer/core/editing/ime/edit_context.cc +@@ -758,6 +758,9 @@ bool EditContext::GetCompositionCharacterBounds(WebVector& bounds) { + bool EditContext::FirstRectForCharacterRange(uint32_t location, + uint32_t length, + gfx::Rect& rect_in_viewport) { ++ gfx::Rect rect_in_css_pixels; ++ bool found_rect = false; ++ + if (HasValidCompositionBounds()) { + WebRange range = this->CompositionRange(); + CHECK_GE(range.StartOffset(), 0); +@@ -772,47 +775,52 @@ bool EditContext::FirstRectForCharacterRange(uint32_t location, + if (length == 0) { + if (start_in_composition == character_bounds_.size()) { + // Zero-width rect after the last character in the composition range +- rect_in_viewport = ++ rect_in_css_pixels = + gfx::Rect(character_bounds_[start_in_composition - 1].right(), + character_bounds_[start_in_composition - 1].y(), 0, + character_bounds_[start_in_composition - 1].height()); + } else { + // Zero-width rect before the next character in the composition range +- rect_in_viewport = ++ rect_in_css_pixels = + gfx::Rect(character_bounds_[start_in_composition].x(), + character_bounds_[start_in_composition].y(), 0, + character_bounds_[start_in_composition].height()); + } + } else { +- gfx::Rect rect = character_bounds_[start_in_composition]; ++ rect_in_css_pixels = character_bounds_[start_in_composition]; + for (size_t i = start_in_composition + 1; i < end_in_composition; ++i) { +- rect.Union(character_bounds_[i]); ++ rect_in_css_pixels.Union(character_bounds_[i]); + } +- +- rect_in_viewport = rect; + } +- return true; ++ found_rect = true; + } + } + + // If we couldn't get a result from the composition bounds then we'll fall + // back to using the selection bounds, since these will generally be close to + // where the composition is happening. +- if (selection_bounds_ != gfx::Rect()) { +- rect_in_viewport = selection_bounds_; +- return true; ++ if (!found_rect && selection_bounds_ != gfx::Rect()) { ++ rect_in_css_pixels = selection_bounds_; ++ found_rect = true; + } + + // If we have neither composition bounds nor selection bounds, we'll fall back + // to using the control bounds. In this case the IME might not be drawn + // exactly in the right spot, but will at least be adjacent to the editable + // region rather than in the corner of the screen. +- if (control_bounds_ != gfx::Rect()) { +- rect_in_viewport = control_bounds_; +- return true; ++ if (!found_rect && control_bounds_ != gfx::Rect()) { ++ rect_in_css_pixels = control_bounds_; ++ found_rect = true; ++ } ++ ++ if (found_rect) { ++ // EditContext's coordinates are in CSS pixels, which need to be converted ++ // to physical pixels before return. ++ rect_in_viewport = gfx::ScaleToEnclosingRect( ++ rect_in_css_pixels, DomWindow()->GetFrame()->DevicePixelRatio()); + } + +- return false; ++ return found_rect; + } + + bool EditContext::HasValidCompositionBounds() const { +diff --git a/third_party/blink/renderer/core/editing/ime/edit_context.h b/third_party/blink/renderer/core/editing/ime/edit_context.h +index 9543f2ead5f09a91a00cbab65f36e8e9f8361cc6..c5fc55476e78a66295e8dc9daa60ddc8a551224c 100644 +--- a/third_party/blink/renderer/core/editing/ime/edit_context.h ++++ b/third_party/blink/renderer/core/editing/ime/edit_context.h +@@ -139,6 +139,8 @@ class CORE_EXPORT EditContext final : public EventTarget, + WebTextInputInfo TextInputInfo() override; + int ComputeWebTextInputNextPreviousFlags() override { return 0; } + WebRange CompositionRange() const override; ++ // Populate `bounds` with the bounds of each item in EditContext's ++ // stored character bounds, scaled to physical pixels. + bool GetCompositionCharacterBounds(WebVector& bounds) override; + WebRange GetSelectionOffsets() const override; + +@@ -154,7 +156,7 @@ class CORE_EXPORT EditContext final : public EventTarget, + void Blur(); + + // Populate |control_bounds| and |selection_bounds| with the bounds fetched +- // from the active EditContext. ++ // from the active EditContext, in physical pixels. + void GetLayoutBounds(gfx::Rect* control_bounds, + gfx::Rect* selection_bounds) override; + +@@ -197,7 +199,7 @@ class CORE_EXPORT EditContext final : public EventTarget, + int end, + bool dispatch_text_update_event = false); + +- // Sets rect_in_viewport to the surrounding rect, in CSS pixels, ++ // Sets rect_in_viewport to the surrounding rect, in physical pixels, + // for the character range specified by `location` and `length`. + // Returns true on success, false on failure (in which case + // rect_in_viewport) is not changed. +@@ -285,6 +287,7 @@ class CORE_EXPORT EditContext final : public EventTarget, + uint32_t selection_start_ = 0; + uint32_t selection_end_ = 0; + ++ // The following bounds are in CSS pixels. + gfx::Rect control_bounds_; + gfx::Rect selection_bounds_; + WebVector character_bounds_; diff --git a/patches/chromium/editcontext_use_empty_control_and_selection_bounds_as_fallbacks.patch b/patches/chromium/editcontext_use_empty_control_and_selection_bounds_as_fallbacks.patch new file mode 100644 index 0000000000..b8a69f11c7 --- /dev/null +++ b/patches/chromium/editcontext_use_empty_control_and_selection_bounds_as_fallbacks.patch @@ -0,0 +1,83 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Daniel Clark +Date: Fri, 2 Aug 2024 18:20:12 +0000 +Subject: Use empty control and selection bounds as fallbacks + +When there are no valid character bounds, +EditContext::FirstRectForCharacterRange is supposed to fall back to +the selection or control bounds if they have been set. Currently we +use an IsEmpty() check to determine if they've been set, but this +actually returns false if the given rect has zero width or height. + +A site may want to set selection to a rect of empty width to indicate +a caret, so this would be skipped as a fallback. + +Fix this by instead checking against the default gfx::Rect to +determine whether a selection or control bounds has been set. + +Bug: 351029417 +Change-Id: I6d519a0e2dab83277b8a5d6df103f0fd8f4a26bf +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5744458 +Reviewed-by: Koji Ishii +Commit-Queue: Dan Clark +Cr-Commit-Position: refs/heads/main@{#1336717} + +diff --git a/third_party/blink/renderer/core/editing/ime/edit_context.cc b/third_party/blink/renderer/core/editing/ime/edit_context.cc +index 71efa75c49ccee79895e5eedf60ccc5d576ac2a3..6c7462ef353f23c8fdfdd9b874f08e506a5c27cd 100644 +--- a/third_party/blink/renderer/core/editing/ime/edit_context.cc ++++ b/third_party/blink/renderer/core/editing/ime/edit_context.cc +@@ -798,7 +798,7 @@ bool EditContext::FirstRectForCharacterRange(uint32_t location, + // If we couldn't get a result from the composition bounds then we'll fall + // back to using the selection bounds, since these will generally be close to + // where the composition is happening. +- if (!selection_bounds_.IsEmpty()) { ++ if (selection_bounds_ != gfx::Rect()) { + rect_in_viewport = selection_bounds_; + return true; + } +@@ -807,7 +807,7 @@ bool EditContext::FirstRectForCharacterRange(uint32_t location, + // to using the control bounds. In this case the IME might not be drawn + // exactly in the right spot, but will at least be adjacent to the editable + // region rather than in the corner of the screen. +- if (!control_bounds_.IsEmpty()) { ++ if (control_bounds_ != gfx::Rect()) { + rect_in_viewport = control_bounds_; + return true; + } +diff --git a/third_party/blink/web_tests/editing/input/edit-context.html b/third_party/blink/web_tests/editing/input/edit-context.html +index 11ef18391ce50f4872e7ae378c3002914354959d..7cd502c38f8b404c18266732baad20ff22634871 100644 +--- a/third_party/blink/web_tests/editing/input/edit-context.html ++++ b/third_party/blink/web_tests/editing/input/edit-context.html +@@ -681,6 +681,32 @@ test(function() { + + test.remove(); + }, 'Testing firstRectForCharacterRange'); ++ ++test(function() { ++ const editContext = new EditContext(); ++ const test = document.createElement("div"); ++ document.body.appendChild(test); ++ test.editContext = editContext; ++ test.focus(); ++ textInputController.insertText("abc"); ++ textInputController.setComposition("def"); ++ ++ assert_array_equals(textInputController.firstRectForCharacterRange(3, 1), [], "No rects are provided if no bounds have been set"); ++ ++ const rect1 = new DOMRect(0, 2, 0, 0); ++ editContext.updateControlBounds(rect1); ++ assert_array_equals(textInputController.firstRectForCharacterRange(3, 1), [0, 2, 0, 0], "If there are no composition bounds and no selection bounds, fall back to control bounds"); ++ ++ const rect2 = new DOMRect(4, 2, 0, 0); ++ editContext.updateSelectionBounds(rect2); ++ assert_array_equals(textInputController.firstRectForCharacterRange(3, 1), [4, 2, 0, 0], "If there are no composition bounds, fall back to selection bounds"); ++ ++ const rect3 = new DOMRect(8, 2, 0, 0); ++ editContext.updateCharacterBounds(3, [rect3, rect3, rect3]); ++ assert_array_equals(textInputController.firstRectForCharacterRange(3, 1), [8, 2, 0, 0], "Character bounds should be used even if control and selection bounds are set"); ++ ++ test.remove(); ++}, 'firstRectForCharacterRange empty control bounds and empty selection bounds fallbacks'); + + + diff --git a/patches/chromium/oom_add_commit_limit_available_to_oom_exception_arguments.patch b/patches/chromium/oom_add_commit_limit_available_to_oom_exception_arguments.patch index e96b5d9aa1..fd1a20bed4 100644 --- a/patches/chromium/oom_add_commit_limit_available_to_oom_exception_arguments.patch +++ b/patches/chromium/oom_add_commit_limit_available_to_oom_exception_arguments.patch @@ -1,8 +1,7 @@ From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Francois Doray Date: Mon, 16 Sep 2024 15:57:01 +0000 -Subject: [oom] Add commit limit/available to OOM exception arguments - (Windows). +Subject: Add commit limit/available to OOM exception arguments (Windows). ~25% of OOM crashes [1] occur for an allocation <10MB when the "system commit remaining" [2] is >1GB. This CL adds the