chromeos (--mus): wires recreation of browser plugins correctly for mus

When a browser plugin is recreated we destroy and create a new
BrowserPlugin. This necessitates reembedding the WindowTreeClient
from the renderer rendering the plugin (because the BrowserPlugin ows
the mus window).

To get the reembeding working required changing
RenderWidgetHostViewGuest to initiate embedding when attached, not
init.

As the renderer rendering the plugin gets a new connection to mus it
needed to be updated. A result of this is the connection to viz is
being dropped, which means ClientLayerTreeFrameSink is seeing it's
connection dropped. The connection is reestablished by way of calling
DidLoseLayerTreeFrameSink().

BUG=755328
TEST=covered by test

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I43838b38bedaf75a341262fcba3befaca77d5c87
Reviewed-on: https://chromium-review.googlesource.com/773104Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517593}

content/browser/browser_plugin/browser_plugin_guest.cc

@@ -851,7 +851,7 @@ void BrowserPluginGuest::OnWillAttachComplete(
   RenderWidgetHostViewGuest* rwhv = static_cast<RenderWidgetHostViewGuest*>(
       web_contents()->GetRenderWidgetHostView());
   if (rwhv)
-    rwhv->RegisterFrameSinkId();
+    rwhv->OnAttached();
   has_render_view_ = true;
 
   RecordAction(base::UserMetricsAction("BrowserPlugin.Guest.Attached"));

content/browser/frame_host/render_widget_host_view_guest.cc

@@ -373,19 +373,6 @@ void RenderWidgetHostViewGuest::SetTooltipText(
     guest_->SetTooltipText(tooltip_text);
 }
 
-void RenderWidgetHostViewGuest::Init() {
-  RenderWidgetHostViewChildFrame::Init();
-
-#if defined(USE_AURA)
-  if (IsUsingMus()) {
-    aura::Env::GetInstance()->ScheduleEmbed(
-        GetWindowTreeClientFromRenderer(),
-        base::BindOnce(&RenderWidgetHostViewGuest::OnGotEmbedToken,
-                       weak_ptr_factory_.GetWeakPtr()));
-  }
-#endif
-}
-
 void RenderWidgetHostViewGuest::SendSurfaceInfoToEmbedderImpl(
     const viz::SurfaceInfo& surface_info,
     const viz::SurfaceSequence& sequence) {
@@ -410,6 +397,18 @@ void RenderWidgetHostViewGuest::SubmitCompositorFrame(
     ClearCompositorSurfaceIfNecessary();
 }
 
+void RenderWidgetHostViewGuest::OnAttached() {
+  RegisterFrameSinkId();
+#if defined(USE_AURA)
+  if (IsUsingMus()) {
+    aura::Env::GetInstance()->ScheduleEmbed(
+        GetWindowTreeClientFromRenderer(),
+        base::BindOnce(&RenderWidgetHostViewGuest::OnGotEmbedToken,
+                       weak_ptr_factory_.GetWeakPtr()));
+  }
+#endif
+}
+
 bool RenderWidgetHostViewGuest::OnMessageReceived(const IPC::Message& msg) {
   if (!platform_view_) {
     // In theory, we can get here if there's a delay between Destroy()

content/browser/frame_host/render_widget_host_view_guest.h

@@ -57,6 +57,9 @@ class CONTENT_EXPORT RenderWidgetHostViewGuest
   bool OnMessageReceivedFromEmbedder(const IPC::Message& message,
                                      RenderWidgetHostImpl* embedder);
 
+  // Called when this RenderWidgetHostViewGuest is attached.
+  void OnAttached();
+
   // RenderWidgetHostView implementation.
   bool OnMessageReceived(const IPC::Message& msg) override;
   void InitAsChild(gfx::NativeView parent_view) override;
@@ -156,8 +159,6 @@ class CONTENT_EXPORT RenderWidgetHostViewGuest
  private:
   friend class RenderWidgetHostView;
 
-  void Init();
-
   void SendSurfaceInfoToEmbedderImpl(
       const viz::SurfaceInfo& surface_info,
       const viz::SurfaceSequence& sequence) override;

content/renderer/browser_plugin/browser_plugin.cc

@@ -250,6 +250,10 @@ void BrowserPlugin::CreateMusWindowAndEmbed(
   DCHECK(renderer_window_tree_client);
   mus_embedded_frame_ =
       renderer_window_tree_client->CreateMusEmbeddedFrame(this, embed_token);
+  if (attached() && local_surface_id_.is_valid()) {
+    mus_embedded_frame_->SetWindowBounds(local_surface_id_,
+                                         FrameRectInPixels());
+  }
 }
 #endif
 

content/renderer/mus/mus_embedded_frame.cc

@@ -29,6 +29,13 @@ void OnEmbedAck(bool success) {
 
 MusEmbeddedFrame::~MusEmbeddedFrame() {
   renderer_window_tree_client_->OnEmbeddedFrameDestroyed(this);
+
+  // If the tree changed the old tree the window was created in was dropped and
+  // the server implictly cleaned up. As such, no need to do cleanup here (and
+  // we don't have a handle to the right WindowTree at this point).
+  if (tree_changed_)
+    return;
+
   // If there is |pending_state_| it means we didn't actually create the window
   // yet and there is nothing to do.
   if (pending_state_)
@@ -40,6 +47,9 @@ MusEmbeddedFrame::~MusEmbeddedFrame() {
 void MusEmbeddedFrame::SetWindowBounds(
     const viz::LocalSurfaceId& local_surface_id,
     const gfx::Rect& bounds) {
+  if (tree_changed_)
+    return;
+
   if (!window_tree()) {
     DCHECK(pending_state_);
     pending_state_->bounds = bounds;
@@ -82,18 +92,26 @@ void MusEmbeddedFrame::CreateChildWindowAndEmbed(
 }
 
 void MusEmbeddedFrame::OnTreeAvailable() {
+  // See header for description, always called before OnTreeWillChange().
+  DCHECK(!tree_changed_);
   std::unique_ptr<PendingState> pending_state = std::move(pending_state_);
   CreateChildWindowAndEmbed(pending_state->token);
   if (pending_state->was_set_window_bounds_called)
     SetWindowBounds(pending_state->local_surface_id, pending_state->bounds);
 }
 
+void MusEmbeddedFrame::OnTreeWillChange() {
+  tree_changed_ = true;
+}
+
 uint32_t MusEmbeddedFrame::GetAndAdvanceNextChangeId() {
   return renderer_window_tree_client_->GetAndAdvanceNextChangeId();
 }
 
 ui::mojom::WindowTree* MusEmbeddedFrame::window_tree() {
-  return renderer_window_tree_client_->tree_.get();
+  // Once |tree_changed_| is true the WindowTree this instance used has changed
+  // and it no longer makes sense to use it (the original window was deleted).
+  return tree_changed_ ? nullptr : renderer_window_tree_client_->tree_.get();
 }
 
 MusEmbeddedFrame::PendingState::PendingState() = default;

content/renderer/mus/mus_embedded_frame.h

@@ -59,6 +59,10 @@ class MusEmbeddedFrame {
   // the MusEmbeddedFrame is created before the WindowTree has been obtained.
   void OnTreeAvailable();
 
+  // Called when the WindowTree held by |renderer_window_tree_client_| is about
+  // to change. This means the renderer was reembeded.
+  void OnTreeWillChange();
+
   // Does the actual embedding.
   void CreateChildWindowAndEmbed(const base::UnguessableToken& token);
 
@@ -72,6 +76,9 @@ class MusEmbeddedFrame {
 
   std::unique_ptr<PendingState> pending_state_;
 
+  // If set, the WindowTree that state was created on has changed.
+  bool tree_changed_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(MusEmbeddedFrame);
 };
 

content/renderer/mus/renderer_window_tree_client.cc

@@ -54,6 +54,10 @@ void RendererWindowTreeClient::Bind(
     ui::mojom::WindowTreeClientRequest request,
     mojom::RenderWidgetWindowTreeClientRequest
         render_widget_window_tree_client_request) {
+  // Bind() may be called multiple times.
+  binding_.Close();
+  render_widget_window_tree_client_binding_.Close();
+
   binding_.Bind(std::move(request));
   render_widget_window_tree_client_binding_.Bind(
       std::move(render_widget_window_tree_client_request));
@@ -171,12 +175,20 @@ void RendererWindowTreeClient::OnEmbed(
     ui::Id focused_window_id,
     bool drawn,
     const base::Optional<viz::LocalSurfaceId>& local_surface_id) {
+  const bool is_reembed = tree_.is_bound();
+  if (is_reembed) {
+    for (MusEmbeddedFrame* frame : embedded_frames_)
+      frame->OnTreeWillChange();
+  }
   root_window_id_ = root->window_id;
+
   tree_ = std::move(tree);
   tree_->SetWindowVisibility(GetAndAdvanceNextChangeId(), root_window_id_,
                              visible_);
-  for (MusEmbeddedFrame* frame : embedded_frames_)
-    frame->OnTreeAvailable();
+  if (!is_reembed) {
+    for (MusEmbeddedFrame* frame : embedded_frames_)
+      frame->OnTreeAvailable();
+  }
 
   if (!pending_layer_tree_frame_sink_callback_.is_null()) {
     RequestLayerTreeFrameSinkInternal(std::move(pending_context_provider_),

testing/buildbot/filters/mojo.fyi.mus.browser_tests.filter

@@ -20,12 +20,9 @@
 
 # TODO: fix, http://crbug.com/755328
 -WebViewTests/WebViewFocusTest.TouchFocusesEmbedder/0
--WebViewTests/WebViewSizeTest.Shim_TestResizeWebviewWithDisplayNoneResizesContent/0
 -WebViewTests/WebViewTest.InterstitialPageFocusedWidget/1
 -WebViewTests/WebViewTest.InterstitialPageRouteEvents/1
 
-# content::WaitForChildFrameSurfaceReady unsupported, https://crbug.com/774269.
--WebViewTests/WebViewTest.ReloadAfterCrash/1
-
 # content::WaitForChildFrameSurfaceReady unsupported, https://crbug.com/763452
 -PDFExtensionTest.ContextMenuCoordinates
+-WebViewTests/WebViewTest.ReloadAfterCrash/1

testing/buildbot/filters/mus.browser_tests.filter

@@ -20,12 +20,9 @@
 
 # TODO: fix, http://crbug.com/755328
 -WebViewTests/WebViewFocusTest.TouchFocusesEmbedder/0
--WebViewTests/WebViewSizeTest.Shim_TestResizeWebviewWithDisplayNoneResizesContent/0
 -WebViewTests/WebViewTest.InterstitialPageFocusedWidget/1
 -WebViewTests/WebViewTest.InterstitialPageRouteEvents/1
 
-# content::WaitForChildFrameSurfaceReady unsupported, https://crbug.com/774269.
--WebViewTests/WebViewTest.ReloadAfterCrash/1
-
 # content::WaitForChildFrameSurfaceReady unsupported, https://crbug.com/763452
 -PDFExtensionTest.ContextMenuCoordinates
+-WebViewTests/WebViewTest.ReloadAfterCrash/1