Commit 33d624de authored by Varun Khaneja's avatar Varun Khaneja Committed by Commit Bot

Remove CPMC from RenderWidgetHost InputEvent observers

Fix: remove ChromePasswordManagerClient from the
InputEventObserver list on WebContentsDestroyed() and
RenderFrameDeleted().

Problem summary: We get an input event when the instance of
ChromePasswordManagerClient is invalid. Reading through the code,
this probably happens because the CPMC is bound to WebContents,
which does get destroyed, but we do not unregister CPMC from the
InputEvent observer list.

Notes:
* This is a speculative fix because I could not reproduce the crash
  locally.
* I am also not sure how to add a test for this condition but I'd
  be happy to add one with guidance.

Bug: 1104919
Change-Id: I3077db25343051c5f2a1e3e905e8abec53556f57
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2309814Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791358}
parent fe753fd9
......@@ -177,6 +177,19 @@ void AddToWidgetInputEventObservers(
widget_host->AddInputEventObserver(observer);
}
// Removes |observer| from the input observers of |widget_host|.
void RemoveFromWidgetInputEventObservers(
content::RenderWidgetHost* widget_host,
content::RenderWidgetHost::InputEventObserver* observer) {
if (!widget_host)
return;
#if defined(OS_ANDROID)
widget_host->RemoveImeInputEventObserver(observer);
#endif
widget_host->RemoveInputEventObserver(observer);
}
#if defined(OS_ANDROID)
void HideSavePasswordInfobar(content::WebContents* web_contents) {
InfoBarService* infobar_service =
......@@ -1179,6 +1192,7 @@ void ChromePasswordManagerClient::DidFinishNavigation(
#if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
password_reuse_detection_manager_.DidNavigateMainFrame(GetLastCommittedURL());
#endif // defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
AddToWidgetInputEventObservers(
web_contents()->GetRenderViewHost()->GetWidget(), this);
#if defined(OS_ANDROID)
......@@ -1200,6 +1214,10 @@ void ChromePasswordManagerClient::WebContentsDestroyed() {
// Other classes may contain callbacks to the Mojo methods. Those callbacks
// don't like to be destroyed earlier than the pipe itself.
content_credential_manager_.DisconnectBinding();
DCHECK(web_contents()->GetRenderViewHost());
RemoveFromWidgetInputEventObservers(
web_contents()->GetRenderViewHost()->GetWidget(), this);
}
#if !defined(OS_ANDROID)
......@@ -1223,6 +1241,14 @@ void ChromePasswordManagerClient::RenderFrameCreated(
render_frame_host->GetView()->GetRenderWidgetHost(), this);
}
void ChromePasswordManagerClient::RenderFrameDeleted(
content::RenderFrameHost* render_frame_host) {
if (!render_frame_host->GetView())
return;
RemoveFromWidgetInputEventObservers(
render_frame_host->GetView()->GetRenderWidgetHost(), this);
}
void ChromePasswordManagerClient::OnInputEvent(
const blink::WebInputEvent& event) {
#if defined(OS_ANDROID)
......
......@@ -293,8 +293,8 @@ class ChromePasswordManagerClient
#if !defined(OS_ANDROID)
void OnPaste() override;
#endif
void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override;
void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
// content::RenderWidgetHost::InputEventObserver overrides.
void OnInputEvent(const blink::WebInputEvent&) override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment