Plugins: Enforce that replacement plugins always succeed initialization.

There's currently ambiguity about what to do if the replacement plugin for Pepper or NPAPI fails initialization. In fact, the replacement plugin never does fail initialization, and it never should.

This patch removes some complexity by enforcing that.

BUG=588624

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

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

4 files changed, 24 insertions, 15 deletions

diff --git a/components/plugins/renderer/webview_plugin.h b/components/plugins/renderer/webview_plugin.h
index a703816..a189ae7 100644
--- a/components/plugins/renderer/webview_plugin.h
+++ b/components/plugins/renderer/webview_plugin.h
@@ -83,6 +83,8 @@ class WebViewPlugin : public blink::WebPlugin,
 
   // WebPlugin methods:
   blink::WebPluginContainer* container() const override;
+  // The WebViewPlugin, by design, never fails to initialize. It's used to
+  // display placeholders and error messages, so it must never fail.
   bool initialize(blink::WebPluginContainer*) override;
   void destroy() override;
 
diff --git a/content/public/renderer/content_renderer_client.h b/content/public/renderer/content_renderer_client.h
index b5c05cd..d602fece 100644
--- a/content/public/renderer/content_renderer_client.h
+++ b/content/public/renderer/content_renderer_client.h
@@ -107,6 +107,8 @@ class CONTENT_EXPORT ContentRendererClient {
 
   // Creates a replacement plugin that is shown when the plugin at |file_path|
   // couldn't be loaded. This allows the embedder to show a custom placeholder.
+  // This may return nullptr. However, if it does return a WebPlugin, it must
+  // never fail to initialize.
   virtual blink::WebPlugin* CreatePluginReplacement(
       RenderFrame* render_frame,
       const base::FilePath& plugin_path);
diff --git a/content/renderer/npapi/webplugin_impl.cc b/content/renderer/npapi/webplugin_impl.cc
index 12ce896..e57ba46 100644
--- a/content/renderer/npapi/webplugin_impl.cc
+++ b/content/renderer/npapi/webplugin_impl.cc
@@ -122,14 +122,19 @@ bool WebPluginImpl::initialize(WebPluginContainer* container) {
       return false;
     }
 
-    // Disable scripting by this plugin before replacing it with the new
-    // one. This plugin also needs destroying, so use destroy(), which will
-    // implicitly disable scripting while un-setting the container.
+    // The replacement plugin, if it exists, must never fail to initialize.
+    container->setPlugin(replacement_plugin);
+    CHECK(replacement_plugin->initialize(container));
+
+    DCHECK(container->plugin() == replacement_plugin);
+    DCHECK(replacement_plugin->container() == container);
+
+    // Since the container now owns the replacement plugin instead of this
+    // object, we must schedule ourselves for deletion. This also implicitly
+    // disables scripting while un-setting the container.
     destroy();
 
-    // Inform the container of the replacement plugin, then initialize it.
-    container->setPlugin(replacement_plugin);
-    return replacement_plugin->initialize(container);
+    return true;
   }
 
   delegate_ = plugin_delegate;
diff --git a/content/renderer/pepper/pepper_webplugin_impl.cc b/content/renderer/pepper/pepper_webplugin_impl.cc
index 7c4e04d..622bfbe 100644
--- a/content/renderer/pepper/pepper_webplugin_impl.cc
+++ b/content/renderer/pepper/pepper_webplugin_impl.cc
@@ -129,17 +129,17 @@ bool PepperWebPluginImpl::initialize(WebPluginContainer* container) {
     if (!replacement_plugin)
       return false;
 
-    // Since we are setting the container to own the replacement plugin, we must
-    // schedule ourselves for deletion.
-    destroy();
+    // The replacement plugin, if it exists, must never fail to initialize.
     container->setPlugin(replacement_plugin);
-    if (!replacement_plugin->initialize(container)) {
-      CHECK(replacement_plugin->container() == nullptr);
-      return false;
-    }
+    CHECK(replacement_plugin->initialize(container));
+
+    DCHECK(container->plugin() == replacement_plugin);
+    DCHECK(replacement_plugin->container() == container);
+
+    // Since the container now owns the replacement plugin instead of this
+    // object, we must schedule ourselves for deletion.
+    destroy();
 
-    CHECK(container->plugin() == replacement_plugin);
-    CHECK(replacement_plugin->container() == container);
     return true;
   }