diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc index 1dddae6e470d9..cabafdec5c662 100644 --- a/content/browser/renderer_host/render_widget_host_view_aura.cc +++ b/content/browser/renderer_host/render_widget_host_view_aura.cc @@ -3417,8 +3417,17 @@ void RenderWidgetHostViewAura::OnUpdateTextInputStateCalled( if (!GetInputMethod()) return; - if (did_update_state) + // OnTextInputTypeChanged() may call out to a third-party TSF IME on Windows, + // which can re-entrantly destroy `this`. + auto weak_this = weak_ptr_factory_.GetWeakPtr(); + + if (did_update_state) { GetInputMethod()->OnTextInputTypeChanged(this); + // `this` may have been deleted inside the IME callout. + if (!weak_this) { + return; + } + } const ui::mojom::TextInputState* state = text_input_manager_->GetTextInputState(); @@ -3460,6 +3469,10 @@ void RenderWidgetHostViewAura::OnUpdateTextInputStateCalled( // Ensure that selection bounds changes are sent to the IME. if (state && state->type != ui::TEXT_INPUT_TYPE_NONE) { text_input_manager->NotifySelectionBoundsChanged(updated_view); + // `this` may have been deleted inside the IME callout. + if (!weak_this) { + return; + } } if (auto* render_widget_host = updated_view->host()) { @@ -3478,8 +3491,14 @@ void RenderWidgetHostViewAura::OnImeCancelComposition( // TextInputManager::GetActiveWidget() as RenderWidgetHostViewAura can call // this method to finish any ongoing composition in response to a mouse down // event. - if (GetInputMethod()) + if (GetInputMethod()) { + auto weak_this = weak_ptr_factory_.GetWeakPtr(); GetInputMethod()->CancelComposition(this); + // `this` may have been deleted inside the IME callout. + if (!weak_this) { + return; + } + } has_composition_text_ = false; } @@ -3513,8 +3532,14 @@ void RenderWidgetHostViewAura::OnTextSelectionChanged( // changed. e.g. When the rendered text is wider than the input field, // deleting the last character won't change the caret bounds but will change // the surrounding text. - if (GetInputMethod()) + if (GetInputMethod()) { + auto weak_this = weak_ptr_factory_.GetWeakPtr(); GetInputMethod()->OnCaretBoundsChanged(this); + // `this` may have been deleted inside the IME callout. + if (!weak_this) { + return; + } + } if (ui::Clipboard::IsSupportedClipboardBuffer( ui::ClipboardBuffer::kSelection)) { diff --git a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc index a7087ef0b3a10..cd6fb26aa46a9 100644 --- a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc +++ b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc @@ -6899,12 +6899,24 @@ class DestroyingMockInputMethod : public ui::MockInputMethod { } } + void OnTextInputTypeChanged(ui::TextInputClient* client) override { + ui::MockInputMethod::OnTextInputTypeChanged(client); + if (on_text_input_type_changed_) { + std::move(on_text_input_type_changed_).Run(); + } + } + void set_on_caret_bounds_changed(base::OnceClosure closure) { on_caret_bounds_changed_ = std::move(closure); } + void set_on_text_input_type_changed(base::OnceClosure closure) { + on_text_input_type_changed_ = std::move(closure); + } + private: base::OnceClosure on_caret_bounds_changed_; + base::OnceClosure on_text_input_type_changed_; }; class RenderWidgetHostViewAuraOnBoundsChangedUAFTest @@ -6961,6 +6973,37 @@ TEST_F(RenderWidgetHostViewAuraOnBoundsChangedUAFTest, raw_view->OnBoundsChanged(gfx::Rect(), gfx::Rect(0, 0, 100, 100)); } +// RWHVA::OnUpdateTextInputStateCalled() calls +// GetInputMethod()->OnTextInputTypeChanged(this), which on Windows reaches +// TSFBridge::OnTextInputTypeChanged() -> ITfThreadMgr::SetFocus(). If the +// active TIP pumps the message queue and the view is destroyed re-entrantly, +// on unwind the function continues to dereference the freed `this` and +// `updated_view`. +TEST_F(RenderWidgetHostViewAuraOnBoundsChangedUAFTest, + DestroyDuringOnTextInputTypeChanged) { + InitViewForFrame(nullptr); + ParentHostView(view_, parent_view_); + // `view_` shares the root window (and thus the InputMethod) with + // `parent_view_`. + ASSERT_EQ(static_cast(input_method_.get()), + GetInputMethod()); + + // Arrange for the view to be synchronously destroyed inside + // OnTextInputTypeChanged, simulating re-entrant destruction triggered by a + // TSF callout that pumps a queued window.close() / renderer-gone task. + input_method_->set_on_text_input_type_changed( + base::BindLambdaForTesting([&]() { + widget_host_ = nullptr; + view_.ExtractAsDangling()->Destroy(); + })); + + ui::mojom::TextInputState state; + state.type = ui::TEXT_INPUT_TYPE_TEXT; + // Dispatching this state notifies `view_` (a TextInputManager observer) via + // OnUpdateTextInputStateCalled(), which calls into the input method above. + GetTextInputManager(view_)->UpdateTextInputState(view_, state); +} + #if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_CHROMEOS) class MockVirtualKeyboardController final : public ui::VirtualKeyboardController {