BrowserPlugin/WebView - Move plugin lifetime to DOM

Source/core/html/HTMLPlugInElement.cpp

Index: Source/core/html/HTMLPlugInElement.cpp
diff --git a/Source/core/html/HTMLPlugInElement.cpp b/Source/core/html/HTMLPlugInElement.cpp
index f4b60adf6c8cbeff0796e7a4d8069f35e615341a..79ac529facc6fc3676c04e4a0843d549aca212e7 100644
--- a/Source/core/html/HTMLPlugInElement.cpp
+++ b/Source/core/html/HTMLPlugInElement.cpp
@@ -142,6 +142,42 @@ void HTMLPlugInElement::updateWidgetIfNecessary()
     updateWidget(CreateOnlyNonNetscapePlugins);
 }
 
+void HTMLPlugInElement::createPluginWithoutRenderer(const String& mimeType)
+{
+    if (m_plugin && (mimeType == m_pluginMimeType))

[eseidel, 2013/11/18 22:26:45]

I'm confused by this check.  It feels like it should be outside of this
function.  This is guarding against calling this twice with the same mimetype,
but I can imagine cases where you'd want this to succeed with the same mimetype
(replacing a plugin with a different one, no?), so it seems this check belongs
at a higher level, or we should aSSERT(!m_plugin) instead of checking.

[wjmaclean, 2013/11/25 17:51:56]

Actually, it might be sensible that, if someone sets the mime-type to the same
value it has at present, we should still create a new plugin. I'll revise
accordingly.

+        return;
+
+    Frame* frame = document().frame();
+    if (!frame->loader().client()->canCreatePluginWithoutRenderer(mimeType))
+        return;
+
+    KURL url;
+    Vector<String> paramNames;
+    Vector<String> paramValues;
+
+    paramNames.append(String("type"));

[eseidel, 2013/11/18 22:26:45]

Shouldn't need an explicit String() wrapper here?

Why do we need to create fake paramNames, the DOM should have them.

[wjmaclean, 2013/11/25 17:51:56]

Done.

+    paramValues.append(mimeType);
+
+    m_plugin = frame->loader().client()->createPlugin(IntSize(), this, url, paramNames, paramValues, mimeType, false, true);
+
+    // Register the bindings with V8 as a scriptable object; normally this is
+    // done in getInstance(), but that is only called after a renderer has been
+    // assigned.
+    if (m_plugin) {
+        m_pluginMimeType = mimeType;
+        // We require a V8 context in order to be able to create the script
+        // instance, but since this may occur during loading we need to create
+        // one here to ensure it exists.
+        v8::HandleScope handleScope(v8::Isolate::GetCurrent());
+        v8::Local<v8::Context> context = ScriptController::mainWorldContext(frame);
+        v8::Context::Scope contextScope(context);
+        m_NPObject = frame->script().createScriptObjectForPluginElement(this);
+        m_pluginWrapper = frame->script().createPluginWrapper(m_plugin.get());
+    } else {
+        m_pluginMimeType = String();
+    }
+}
+
 void HTMLPlugInElement::detach(const AttachContext& context)
 {
     // Update the widget the next time we attach (detaching destroys the plugin).
@@ -149,7 +185,18 @@ void HTMLPlugInElement::detach(const AttachContext& context)
     if (renderer() && !useFallbackContent())
         setNeedsWidgetUpdate(true);
 
-    resetInstance();
+    if (m_plugin) {
+        RenderEmbeddedObject* renderer = renderEmbeddedObject();
+        if (renderer)
+            renderer->configureWidget(RenderWidget::ConfigureOnDetach);
+        else
+            ASSERT(!m_plugin->parent());
+
+        if (!m_plugin->pluginShouldPersist()) {
+            m_plugin.clear();
+            resetInstance();
+        }
+    }
 
     if (m_isCapturingMouseEvents) {
         if (Frame* frame = document().frame())
@@ -436,8 +483,12 @@ bool HTMLPlugInElement::loadPlugin(const KURL& url, const String& mimeType, cons
     m_loadedUrl = url;
 
     IntSize contentSize = roundedIntSize(LayoutSize(renderer->contentWidth(), renderer->contentHeight()));
-    bool loadManually = document().isPluginDocument() && !document().containsPlugins() && toPluginDocument(document()).shouldLoadPluginManually();
-    RefPtr<Widget> widget = frame->loader().client()->createPlugin(contentSize, this, url, paramNames, paramValues, mimeType, loadManually);
+    RefPtr<Widget> widget = plugin();
+
+    if (!widget) {
+        bool loadManually = document().isPluginDocument() && !document().containsPlugins() && toPluginDocument(document()).shouldLoadPluginManually();

[eseidel, 2013/11/18 22:26:45]

There should only be one way to create a plugin.  This code shouldn't need to be
sensitive to there being a second way. :(

[wjmaclean, 2013/11/25 17:51:56]

Done.

+        widget = frame->loader().client()->createPlugin(contentSize, this, url, paramNames, paramValues, mimeType, loadManually);
+    }
 
     if (!widget) {
         if (!renderer->showsUnavailablePluginIndicator())
@@ -445,7 +496,12 @@ bool HTMLPlugInElement::loadPlugin(const KURL& url, const String& mimeType, cons
         return false;
     }
 
-    renderer->setWidget(widget);
+    // FIXME: What happens here if our widget *isn't* a plugin container?

[eseidel, 2013/11/18 22:26:45]

That can't happen.  We're an HTMLPluginElement, we should only have Plugin
subclasses of Widget.  We can ASSERT that here.

[wjmaclean, 2013/11/25 17:51:56]

Done.

+    if (widget->isPluginContainer())
+        setPlugin(widget);
+    else
+        ASSERT_NOT_REACHED();
+    renderer->configureWidget(RenderWidget::ConfigureOnAttach);
     document().setContainsPlugins();
     setNeedsStyleRecalc(LocalStyleChange, StyleChangeFromRenderer);
     return true;
@@ -501,4 +557,14 @@ bool HTMLPlugInElement::pluginIsLoadable(const KURL& url, const String& mimeType
     return frame->loader().mixedContentChecker()->canRunInsecureContent(document().securityOrigin(), url);
 }
 
+void HTMLPlugInElement::setPlugin(PassRefPtr<Widget> plugin)
+{
+    m_plugin = plugin;
+}
+
+PassRefPtr<Widget> HTMLPlugInElement::plugin()
+{
+    return m_plugin;
+}
+
 }