Reset Text Input State for RenderWidgetHostView before RenderWidgetHost Detaches from Delegate

content/browser/browser_plugin/browser_plugin_guest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/browser_plugin/browser_plugin_guest.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 
@@ skipping 18 matching lines @@
 #include "content/browser/renderer_host/render_widget_host_view_base.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/browser/web_contents/web_contents_view_guest.h"
 #include "content/common/browser_plugin/browser_plugin_constants.h"
 #include "content/common/browser_plugin/browser_plugin_messages.h"
 #include "content/common/content_constants_internal.h"
 #include "content/common/drag_messages.h"
 #include "content/common/host_shared_bitmap_manager.h"
 #include "content/common/input_messages.h"
 #include "content/common/site_isolation_policy.h"
+#include "content/common/text_input_state.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_plugin_guest_manager.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/guest_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/browser_plugin_guest_mode.h"
 #include "content/public/common/drop_data.h"
@@ skipping 720 matching lines @@
   rwhv->RegisterSurfaceNamespaceId();
   has_render_view_ = true;
 
   RecordAction(base::UserMetricsAction("BrowserPlugin.Guest.Attached"));
 }
 
 void BrowserPluginGuest::OnDetach(int browser_plugin_instance_id) {
   if (!attached())
     return;
 
+  // The BrowserPluginGuest loses the link to |owner_web_contents_|. Therefore,
+  // the RenderWidgetHostViewGuest will not get a chance to update its text
+  // input state which is tracked by |owner_web_contents_| (crbug.com/602144).

[lazyboy, 2016/04/12 04:11:54]

Generally you refer to a crbug when there's an issue in the current code and you
want to fix it later, possibly with adding TODO. This code seems like it will
fix 602144, so *probably* no need to refer to the crbug link here?

Ideally, we should be "detaching" the inner web_contents from the outer
web_contents through WebContentsImpl. There isn't a mechanism right now, but I
think eventually we should (that is a long standing TODO of mine).
Can we add a TODO saying we should perform the detach-ment through
WebContentsImpl, so that WebContentsImpl can take care of its lingering link to
incorrect TextInputState?

[EhsanK, 2016/04/12 15:06:49]

Yes this is supposed to be part of the fix for crbug.com/602144.I was trying to
put more context into the comment explaining why we are doing this here. But I
see your point.
I agree. I was discussing this with lfg@ yesterday. I also think it is not the
best way to handle a detach. Specifically given that (from what I see), the
outer WebContents outlives the inner one and also BrowserPluginGuest. So we
should have a reference to it (and we do but owner_web_contents() returns
nullptr when |detached_| is false) for as long as we need.
I will and I will also report a bug to track this.

+  OnTextInputStateChanged(TextInputState());
+
   // This tells BrowserPluginGuest to queue up all IPCs to BrowserPlugin until
   // it's attached again.
   attached_ = false;
 
   RenderWidgetHostViewChildFrame* rwhv =
       static_cast<RenderWidgetHostViewChildFrame*>(
           web_contents()->GetRenderWidgetHostView());
   // If the guest is terminated, our host may already be gone.
   if (rwhv)
     rwhv->UnregisterSurfaceNamespaceId();
@@ skipping 213 matching lines @@
           range, character_bounds);
 }
 #endif
 
 void BrowserPluginGuest::SetContextMenuPosition(const gfx::Point& position) {
   if (delegate_)
     delegate_->SetContextMenuPosition(position);
 }
 
 }  // namespace content