[ MimeHandlerView ] Cleaning up MimeHandlerViewEmbedder when disallowed

The process of creating MimeHandlerView is:
  * Navigation to resource starts
  * MimeHandlerViewEmbedder is created
  * HTMLPlugInElement::UpdatePlugin is called
  * MimeHandlerViewContainerManager is notified
  * HTMLPlugInElement creates a frame
  * MimeHandlerViewEmbedder observes the new frame and attaches
    GuestView, and destroys itself.

These steps are legitimately disturbed when the embedder is not
authorized to load plugins (e.g., in <webview> when using permissions).
In such cases, the update from HTMLPlugInElement does not reach the
MimeHandlerViewContainerManager and also a plugin frame is not created.
Therefore, MimeHandlerViewEmbedder does not get destroyed.

This CL will change the update process so that even for blocked plugins
the MHVCM is notified (of the failure) which gives it a chance to notify
the browser in turn and clean up the MimeHandlerViewEmbedder.

In line with the change, the test:
  WebViewPluginTest.TestLoadPluginInternalResource
is modified to assert no MimeHandlerViewEmbedders are lying around.

Bug: 967045
Change-Id: Ic63a980883240fe8098a4ee8d5e3ae3b9cc46415
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1629071Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663982}

chrome/browser/apps/guest_view/web_view_browsertest.cc

@@ -92,6 +92,7 @@
 #include "extensions/browser/api/declarative_webrequest/webrequest_constants.h"
 #include "extensions/browser/api/extensions_api_client.h"
 #include "extensions/browser/app_window/native_app_window.h"
+#include "extensions/browser/guest_view/mime_handler_view/mime_handler_view_embedder.h"
 #include "extensions/browser/guest_view/web_view/web_view_guest.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extensions_client.h"
@@ -3341,7 +3342,13 @@ IN_PROC_BROWSER_TEST_F(WebViewPluginTest, TestLoadPluginInternalResource) {
                                                                 true);
 
   TestHelper("testPluginLoadInternalResource", "web_view/shim", NO_TEST_SERVER);
+  // Sanity check to ensure no GuestView was created.
+  for (auto* guest_wc : GetEmbedderWebContents()->GetInnerWebContents()) {
+    EXPECT_FALSE(extensions::MimeHandlerViewEmbedder::Get(
+        guest_wc->GetMainFrame()->GetFrameTreeNodeId()));
+  }
 }
+
 #endif  // BUILDFLAG(ENABLE_PLUGINS)
 
 class WebViewCaptureTest : public WebViewTest {

chrome/renderer/chrome_content_renderer_client.cc

@@ -613,15 +613,18 @@ bool ChromeContentRendererClient::IsPluginHandledExternally(
   // necessary here (see https://crbug.com/965747). For now, returning false
   // should take us to CreatePlugin after HTMLPlugInElement which is called
   // through HTMLPlugInElement::LoadPlugin code path.
-  if ((plugin_info->status != chrome::mojom::PluginStatus::kAllowed &&
-       plugin_info->status !=
-           chrome::mojom::PluginStatus::kPlayImportantContent) ||
-      !ChromeExtensionsRendererClient::MaybeCreateMimeHandlerView(
-          plugin_element, original_url, plugin_info->actual_mime_type,
-          plugin_info->plugin)) {
+  if (plugin_info->status != chrome::mojom::PluginStatus::kAllowed &&
+      plugin_info->status !=
+          chrome::mojom::PluginStatus::kPlayImportantContent) {
+    // We could get here when a MimeHandlerView is loaded inside a <webview>
+    // which is using permissions API (see WebViewPluginTests).
+    ChromeExtensionsRendererClient::DidBlockMimeHandlerViewForDisallowedPlugin(
+        plugin_element);
     return false;
   }
-  return true;
+  return ChromeExtensionsRendererClient::MaybeCreateMimeHandlerView(
+      plugin_element, original_url, plugin_info->actual_mime_type,
+      plugin_info->plugin);
 #else
   return false;
 #endif

chrome/renderer/extensions/chrome_extensions_renderer_client.cc

@@ -318,6 +318,16 @@ ChromeExtensionsRendererClient::CreateBrowserPluginDelegate(
                                                   original_url);
 }
 
+// static
+void ChromeExtensionsRendererClient::DidBlockMimeHandlerViewForDisallowedPlugin(
+    const blink::WebElement& plugin_element) {
+  extensions::MimeHandlerViewContainerManager::Get(
+      content::RenderFrame::FromWebFrame(
+          plugin_element.GetDocument().GetFrame()),
+      true /* create_if_does_not_exist */)
+      ->DidBlockMimeHandlerViewForDisallowedPlugin(plugin_element);
+}
+
 // static
 bool ChromeExtensionsRendererClient::MaybeCreateMimeHandlerView(
     const blink::WebElement& plugin_element,

chrome/renderer/extensions/chrome_extensions_renderer_client.h

@@ -93,6 +93,8 @@ class ChromeExtensionsRendererClient
       const content::WebPluginInfo& info,
       const std::string& mime_type,
       const GURL& original_url);
+  static void DidBlockMimeHandlerViewForDisallowedPlugin(
+      const blink::WebElement& plugin_element);
   static bool MaybeCreateMimeHandlerView(
       const blink::WebElement& plugin_element,
       const GURL& resource_url,

extensions/browser/guest_view/extensions_guest_view_message_filter.cc

@@ -24,6 +24,7 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/guest_view/mime_handler_view/mime_handler_stream_manager.h"
 #include "extensions/browser/guest_view/mime_handler_view/mime_handler_view_constants.h"
+#include "extensions/browser/guest_view/mime_handler_view/mime_handler_view_embedder.h"
 #include "extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h"
 #include "extensions/browser/guest_view/web_view/web_view_content_script_manager.h"
 #include "extensions/browser/guest_view/web_view/web_view_guest.h"
@@ -126,6 +127,22 @@ void ExtensionsGuestViewMessageFilter::CreateMimeHandlerViewGuest(
                      false));
 }
 
+void ExtensionsGuestViewMessageFilter::ReadyToCreateMimeHandlerView(
+    int32_t render_frame_id,
+    bool success) {
+  if (!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) {
+    base::PostTaskWithTraits(
+        FROM_HERE, {content::BrowserThread::UI},
+        base::BindOnce(
+            &ExtensionsGuestViewMessageFilter::ReadyToCreateMimeHandlerView,
+            this, render_frame_id, success));
+    return;
+  }
+  auto* rfh =
+      content::RenderFrameHost::FromID(render_process_id_, render_frame_id);
+  if (auto* mhve = MimeHandlerViewEmbedder::Get(rfh->GetFrameTreeNodeId()))
+    mhve->ReadyToCreateMimeHandlerView(success);
+}
 void ExtensionsGuestViewMessageFilter::CreateMimeHandlerViewGuestOnUIThread(
     int render_frame_id,
     const std::string& view_id,

extensions/browser/guest_view/extensions_guest_view_message_filter.h

@@ -82,6 +82,8 @@ class ExtensionsGuestViewMessageFilter
       int32_t element_instance_id,
       const gfx::Size& element_size,
       mime_handler::BeforeUnloadControlPtr before_unload_control) override;
+  void ReadyToCreateMimeHandlerView(int32_t render_frame_id,
+                                    bool success) override;
 
   void CreateMimeHandlerViewGuestOnUIThread(
       int32_t render_frame_id,

extensions/browser/guest_view/mime_handler_view/mime_handler_view_embedder.cc

@@ -33,6 +33,17 @@ EmbedderMap* GetMimeHandlerViewEmbeddersMap() {
 }
 }  // namespace
 
+// static
+MimeHandlerViewEmbedder* MimeHandlerViewEmbedder::Get(
+    int32_t frame_tree_node_id) {
+  const auto& map = *GetMimeHandlerViewEmbeddersMap();
+  auto it = map.find(frame_tree_node_id);
+  if (it == map.cend())
+    return nullptr;
+  return it->second.get();
+}
+
+// static
 void MimeHandlerViewEmbedder::Create(int32_t frame_tree_node_id,
                                      const GURL& resource_url,
                                      const std::string& mime_type,
@@ -96,6 +107,13 @@ void MimeHandlerViewEmbedder::RenderFrameCreated(
       render_frame_host_->GetLastCommittedURL() != resource_url_) {
     return;
   }
+  if (!ready_to_create_mime_handler_view_) {
+    // Renderer notifies the browser about creating MimeHandlerView right after
+    // HTMLPlugInElement::RequestObject, which is before the plugin element is
+    // navigated.
+    GetMimeHandlerViewEmbeddersMap()->erase(frame_tree_node_id_);
+    return;
+  }
   outer_contents_frame_tree_node_id_ = render_frame_host->GetFrameTreeNodeId();
   element_instance_id_ = render_frame_host->GetRoutingID();
   // This suggests that a same-origin child frame is created under the
@@ -185,4 +203,12 @@ MimeHandlerViewEmbedder::GetContainerManager() {
   }
   return container_manager_.get();
 }
+
+void MimeHandlerViewEmbedder::ReadyToCreateMimeHandlerView(
+    bool ready_to_create_mime_handler_view) {
+  ready_to_create_mime_handler_view_ = ready_to_create_mime_handler_view;
+  if (!ready_to_create_mime_handler_view_)
+    GetMimeHandlerViewEmbeddersMap()->erase(frame_tree_node_id_);
+}
+
 }  // namespace extensions

extensions/browser/guest_view/mime_handler_view/mime_handler_view_embedder.h

@@ -32,6 +32,10 @@ namespace extensions {
 //.  - the embedder or the <iframe> are removed from DOM.
 class MimeHandlerViewEmbedder : public content::WebContentsObserver {
  public:
+  // Returns the instances associated with an ongoing navigation in a frame
+  // identified by |frame_tree_node_id|.
+  static MimeHandlerViewEmbedder* Get(int32_t frame_tree_node_id);
+
   static void Create(int32_t frame_tree_node_id,
                      const GURL& resource_url,
                      const std::string& mime_type,
@@ -46,6 +50,8 @@ class MimeHandlerViewEmbedder : public content::WebContentsObserver {
   void DidStartNavigation(content::NavigationHandle* handle) override;
   void ReadyToCommitNavigation(content::NavigationHandle* handle) override;
 
+  void ReadyToCreateMimeHandlerView(bool result);
+
  private:
   MimeHandlerViewEmbedder(int32_t frame_tree_node_id,
                           const GURL& resource_url,
@@ -82,6 +88,8 @@ class MimeHandlerViewEmbedder : public content::WebContentsObserver {
 
   const std::string internal_id_;
 
+  bool ready_to_create_mime_handler_view_ = false;
+
   base::WeakPtrFactory<MimeHandlerViewEmbedder> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(MimeHandlerViewEmbedder);

extensions/common/mojo/guest_view.mojom

@@ -30,6 +30,13 @@ interface GuestView {
       int32 element_instance_id,
       gfx.mojom.Size element_size,
       extensions.mime_handler.BeforeUnloadControl? before_unload_control);
+
+  // Notifies the browser whether or not now is a good time to start loading the
+  // MimeHandlerView. |routing_id| identifies the embedder frame. If |success|
+  // is false, then MimeHandlerViewEmbedder should destroy itself. When
+  // |success| is true the MimeHandlerViewEmbedder should proceed with attaching
+  // the GuestView.
+  ReadyToCreateMimeHandlerView(int32 routing_id, bool success);
 };
 
 // An interface implemented by the renderer which is used for creating a

extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.cc

@@ -145,6 +145,11 @@ PostMessageSupport::Delegate* PostMessageSupport::Delegate::FromWebLocalFrame(
   return mime_handlers.front();
 }
 
+// static
+mojom::GuestView* MimeHandlerViewContainerBase::GuestView() {
+  return GetGuestView();
+}
+
 // static
 std::vector<MimeHandlerViewContainerBase*>
 MimeHandlerViewContainerBase::FromRenderFrame(

extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.h

@@ -46,6 +46,8 @@ class MimeHandlerViewContainerBase : public blink::WebAssociatedURLLoaderClient,
 
   ~MimeHandlerViewContainerBase() override;
 
+  static mojom::GuestView* GuestView();
+
   // TODO(ekaramad): Remove this and make MimeHandlerViewContainerManager of
   // |render_frame| hold on to the list of MimeHandlerViewContainerBase.
   static std::vector<MimeHandlerViewContainerBase*> FromRenderFrame(

extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_manager.cc

@@ -109,6 +109,18 @@ bool MimeHandlerViewContainerManager::CreateFrameContainer(
   return true;
 }
 
+void MimeHandlerViewContainerManager::
+    DidBlockMimeHandlerViewForDisallowedPlugin(
+        const blink::WebElement& plugin_element) {
+  if (IsManagedByContainerManager(plugin_element)) {
+    // This is the one injected by HTML string. Return true so that the
+    // HTMLPlugInElement creates a child frame to be used as the outer
+    // WebContents frame.
+    MimeHandlerViewContainerBase::GuestView()->ReadyToCreateMimeHandlerView(
+        render_frame()->GetRoutingID(), false);
+  }
+}
+
 v8::Local<v8::Object> MimeHandlerViewContainerManager::GetScriptableObject(
     const blink::WebElement& plugin_element,
     v8::Isolate* isolate) {
@@ -295,6 +307,8 @@ bool MimeHandlerViewContainerManager::IsManagedByContainerManager(
       base::ToUpperASCII(plugin_element.GetAttribute("internalid").Utf8()) ==
           internal_id_) {
     plugin_element_ = plugin_element;
+    MimeHandlerViewContainerBase::GuestView()->ReadyToCreateMimeHandlerView(
+        render_frame()->GetRoutingID(), true);
   }
   return plugin_element_ == plugin_element;
 }

extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_manager.h

@@ -66,6 +66,10 @@ class MimeHandlerViewContainerManager
                             const GURL& resource_url,
                             const std::string& mime_type,
                             const content::WebPluginInfo& plugin_info);
+  // Called to notify about a failed plugin load; this could happen if a
+  // <webview> with permissions API tries to load a plugin.
+  void DidBlockMimeHandlerViewForDisallowedPlugin(
+      const blink::WebElement& plugin_element);
   // A wrapper for custom postMessage scripts. There should already be a
   // MimeHandlerViewFrameContainer for |plugin_element|.
   v8::Local<v8::Object> GetScriptableObject(