New issue
Advanced search Search tips

Issue 602144 link

Starred by 2 users
Status: Fixed
Owner:
ekaramad@chromium.org
Closed: Nov 2016
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocking:
issue 601570



Sign in to add a comment

Destroying Inner WebContents Does Not Reset TextInputState in Outer WebContents.

Project Member Reported by ekaramad@chromium.org, Apr 11 2016 
In the destructor of WebContentsImpl, all the RWH are first detached and then destroyed. This leads to RWHVs not being able to reset the text input state in the outer WebContentsImpl through a call to RenderWidgetHostViewBase::NotifyHostDelegateAboutShutdown(). Eventually, this triggers a DCHECK in WebContentsImpl::GetTextInputState due to a missing |view_with_active_text_input_| but a |text_input_state_| which is not none.

What steps will reproduce the problem?
(1) Create an empty app with <webview> permissions.
(2) Navigate the <webview> to a page with a text input field.
(3) Focus the text input field.
(4) Detach the <webview>.

What is the expected output?
The DCHECK in WebContentsImpl::GetTextInputState hits.

What do you see instead?
The DCHECK should not hit. This is due to WebContentsImpl not updating its input state.

Comment 1 by ekaramad@chromium.org, Apr 11 2016 

Another reason causing this bug is the in BrowserPluginGuest, the reference to |owner_web_contents_| is lost before the |guest_web_contents_| are destroyed. In OOPIF world, the inner WebContents (guest) will lose its access to outer WebContents (embedder) during this destruction; hence no text input state clean up.

Comment 2 by ekaramad@chromium.org, Apr 12 2016

Labels: -Type-Bug Type-Feature
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 14 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5292d041d34d49e8c3e851a0e21504a983e5be57

commit 5292d041d34d49e8c3e851a0e21504a983e5be57
Author: ekaramad <ekaramad@chromium.org>
Date: Thu Apr 14 14:30:52 2016

Reset Text Input State for RenderWidgetHostView before RenderWidgetHost Detaches from Delegate

During the destruction of WebContentsImpl, the RWHs are first detached and then destroyed. If some RWHV
has an active input field, then it will never get a chance to notify the WebContentsImpl about
losing being destroyed and the WebContentsImpl will track the wrong text input state. This will
cause DCHECK in WebContentsImpl::GetTextInputState being triggered, specifically, for an outer WebContents
which might outlive the WebContents being destroyed.

BUG= 602144 ,  601570 

Review URL: https://codereview.chromium.org/1879453002

Cr-Commit-Position: refs/heads/master@{#387311}

[modify] https://crrev.com/5292d041d34d49e8c3e851a0e21504a983e5be57/chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc
[modify] https://crrev.com/5292d041d34d49e8c3e851a0e21504a983e5be57/chrome/test/data/extensions/platform_apps/web_view/text_input_state/window.js
[modify] https://crrev.com/5292d041d34d49e8c3e851a0e21504a983e5be57/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/5292d041d34d49e8c3e851a0e21504a983e5be57/content/browser/renderer_host/render_widget_host_impl.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c

commit 6bb171a404c2e9a951a1c8c980fc316da4b7fd7c
Author: ekaramad <ekaramad@chromium.org>
Date: Fri Apr 15 22:09:36 2016

Revert of Browser Side Text Input State Tracking for OOPIF (Manual).

The original patch (https://codereview.chromium.org/1652483002/) led to
many regressions. This CL is a manual revert of that patch, plus several
dependent CLs that landed subsequently.

Revert "Browser Side Text Input State Tracking for OOPIF."
This reverts commit 2bd4a2f0c0f114979d47f4498ea1bce9b091591e.

Revert "Disable WebViewTextInputStateInteractiveTest.TopLevelWebContentsTracksCorrectly on Win"
This reverts commit ce2d8a33e2f0b9c742a67ba47618eb9e9172d825.

Revert "Disable WebViewTextInputStateInteractiveTest.TopLevelWebContentsTracksCorrectly on Mac"
This reverts commit 83e4dfa6bc12f0fb1a5da73fc82f19776a158c75.

Revert "Initialize TextInputState::TextInputState::show_ime_if_needed in the constructor."
This reverts commit 0c15a6bce78d7dc48bdaac09d90dc8cd55893b6f.

Revert "Add the missing text input state tracking code to RenderWidgetHostViewMac."
This reverts commit 46248c7ed4872cffc4ebb0b9da458678f7d7e7a3.

Revert "Reset Text Input State for RenderWidgetHostView before RenderWidgetHost Detaches from Delegate"
This reverts commit 5292d041d34d49e8c3e851a0e21504a983e5be57.

BUG= 578168 ,  546645 ,  601570 ,  601738 ,  602144 ,  602488 , 602926,  602954 , 603209,  603676 , 603886
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/1889313002

Cr-Commit-Position: refs/heads/master@{#387715}

[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc
[delete] https://crrev.com/668d6c81de9e5b821ed96671f4f435093fafa404/chrome/test/data/extensions/platform_apps/web_view/text_input_state/background.js
[delete] https://crrev.com/668d6c81de9e5b821ed96671f4f435093fafa404/chrome/test/data/extensions/platform_apps/web_view/text_input_state/guest.html
[delete] https://crrev.com/668d6c81de9e5b821ed96671f4f435093fafa404/chrome/test/data/extensions/platform_apps/web_view/text_input_state/manifest.json
[delete] https://crrev.com/668d6c81de9e5b821ed96671f4f435093fafa404/chrome/test/data/extensions/platform_apps/web_view/text_input_state/window.html
[delete] https://crrev.com/668d6c81de9e5b821ed96671f4f435093fafa404/chrome/test/data/extensions/platform_apps/web_view/text_input_state/window.js
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/DEPS
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/browser_plugin/browser_plugin_guest.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/frame_host/interstitial_page_impl.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/frame_host/interstitial_page_impl.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/frame_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/frame_host/render_widget_host_view_child_frame_unittest.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/frame_host/render_widget_host_view_guest.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/frame_host/render_widget_host_view_guest_unittest.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_delegate.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_delegate.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_view_mus.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/renderer_host/render_widget_host_view_mus.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/site_per_process_browsertest.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/browser/web_contents/web_contents_impl.h
[delete] https://crrev.com/668d6c81de9e5b821ed96671f4f435093fafa404/content/common/text_input_state.cc
[delete] https://crrev.com/668d6c81de9e5b821ed96671f4f435093fafa404/content/common/text_input_state.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/common/view_messages.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/content_common.gypi
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/public/test/browser_test_utils.h
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/renderer/render_widget.cc
[delete] https://crrev.com/668d6c81de9e5b821ed96671f4f435093fafa404/content/test/data/textinput/page_with_input.html
[delete] https://crrev.com/668d6c81de9e5b821ed96671f4f435093fafa404/content/test/data/textinput/page_with_input_iframeX2_input.html
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/test/test_render_view_host.cc
[modify] https://crrev.com/6bb171a404c2e9a951a1c8c980fc316da4b7fd7c/content/test/test_render_view_host.h
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 18 2016

Labels: merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46b778341feee9da0294c6138d860ad20dffca90

commit 46b778341feee9da0294c6138d860ad20dffca90
Author: ekaramad <ekaramad@chromium.org>
Date: Mon Apr 18 20:44:49 2016

Revert "Browser Side Text Input State Tracking for OOPIF."

The original patch (https://codereview.chromium.org/1652483002/) led to
many regressions. This CL is a manual revert of that patch, plus several
dependent CLs that landed subsequently.

Revert "Browser Side Text Input State Tracking for OOPIF."
This reverts commit 2bd4a2f0c0f114979d47f4498ea1bce9b091591e.

Revert "Initialize TextInputState::TextInputState::show_ime_if_needed in the constructor."
This reverts commit 0c15a6bce78d7dc48bdaac09d90dc8cd55893b6f.

Revert "Add the missing text input state tracking code to RenderWidgetHostViewMac."
This reverts commit 73a3921eff94c55d259dac5d1536a6b4bc88345a.

BUG= 578168 ,  546645 ,  601424 ,  601570 ,  601738 ,  602144 ,  602488 , 602926,  602954 , 603209,  603676 , 603886
NOTRY=true
NOPRESUBMIT=true

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/1898803002

Cr-Commit-Position: refs/branch-heads/2704@{#108}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc
[delete] https://crrev.com/73ebab994224d08a93500fe8dc8e328ca0ac58ec/chrome/test/data/extensions/platform_apps/web_view/text_input_state/background.js
[delete] https://crrev.com/73ebab994224d08a93500fe8dc8e328ca0ac58ec/chrome/test/data/extensions/platform_apps/web_view/text_input_state/guest.html
[delete] https://crrev.com/73ebab994224d08a93500fe8dc8e328ca0ac58ec/chrome/test/data/extensions/platform_apps/web_view/text_input_state/manifest.json
[delete] https://crrev.com/73ebab994224d08a93500fe8dc8e328ca0ac58ec/chrome/test/data/extensions/platform_apps/web_view/text_input_state/window.html
[delete] https://crrev.com/73ebab994224d08a93500fe8dc8e328ca0ac58ec/chrome/test/data/extensions/platform_apps/web_view/text_input_state/window.js
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/DEPS
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/browser_plugin/browser_plugin_guest.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/frame_host/interstitial_page_impl.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/frame_host/interstitial_page_impl.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/frame_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/frame_host/render_widget_host_view_child_frame_unittest.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/frame_host/render_widget_host_view_guest.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/frame_host/render_widget_host_view_guest_unittest.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_delegate.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_delegate.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_view_mus.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/renderer_host/render_widget_host_view_mus.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/site_per_process_browsertest.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/browser/web_contents/web_contents_impl.h
[delete] https://crrev.com/73ebab994224d08a93500fe8dc8e328ca0ac58ec/content/common/text_input_state.cc
[delete] https://crrev.com/73ebab994224d08a93500fe8dc8e328ca0ac58ec/content/common/text_input_state.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/common/view_messages.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/content_common.gypi
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/public/test/browser_test_utils.h
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/renderer/render_widget.cc
[delete] https://crrev.com/73ebab994224d08a93500fe8dc8e328ca0ac58ec/content/test/data/textinput/page_with_input.html
[delete] https://crrev.com/73ebab994224d08a93500fe8dc8e328ca0ac58ec/content/test/data/textinput/page_with_input_iframeX2_input.html
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/test/test_render_view_host.cc
[modify] https://crrev.com/46b778341feee9da0294c6138d860ad20dffca90/content/test/test_render_view_host.h

Comment 6 by ekaramad@chromium.org, Apr 27 2016

Labels: -Pri-1 Pri-2
Setting priority to 2 since the issue causing the error is reverted. However, the bug will remain open since the issue related to this needs to be tracked in the relanding of https://codereview.chromium.org/1652483002/.

Comment 7 by ekaramad@chromium.org, Nov 22 2016

Status: Fixed (was: Started)
This is an old IME bug whose original CL is now reverted. We can safely close this.
►

Sign in to add a comment