Javascript in foreground tabs should not wait synchronously for plug-ins to load
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Sep 2012 19:08:11 +0000 (19:08 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Sep 2012 19:08:11 +0000 (19:08 +0000)
<rdar://problem/12067415> and https://bugs.webkit.org/show_bug.cgi?id=96167

Reviewed by Geoff Garen.

Source/WebKit2:

Synchronously waiting for initialization to complete when javascript accesses the plug-in script object severely
reduces the effectiveness of having an asynchronous NPP_New.

Such as with background tabs that have never been viewed, we already have cases where JS calls into the plug-in
element fail because we haven't bothered to initialize the plug-in.

We get a huge win by expanding that to foreground tabs that simply haven't finished initializing their plug-ins.

* WebProcess/Plugins/PluginView.cpp:
(WebKit::PluginView::scriptObject): If initialization is not complete just return 0 - They can get at the script object later.

Remove the notion of "wait for asynchronous initialization" altogether:
* WebProcess/Plugins/Netscape/NetscapePlugin.h:
(NetscapePlugin):
* WebProcess/Plugins/PDF/BuiltInPDFView.h:
(BuiltInPDFView):
* WebProcess/Plugins/Plugin.h:
(Plugin):
* WebProcess/Plugins/PluginProxy.cpp:
* WebProcess/Plugins/PluginProxy.h:
(PluginProxy):

Tools:

Enhance the "Slow NPP_New" plug-in to also be able to return properties to javascript.

* DumpRenderTree/TestNetscapePlugIn/Tests/SlowNPPNew.cpp:
(PluginObject):
(SlowNPPNew::PluginObject::PluginObject):
(SlowNPPNew::PluginObject::~PluginObject):
(SlowNPPNew::PluginObject::hasProperty):
(SlowNPPNew::PluginObject::getProperty):
(SlowNPPNew::NPP_GetValue):
(SlowNPPNew):

LayoutTests:

* platform/mac-wk2/plugins/script-object-access-fails-during-slow-initialization-expected.txt: Added.
* platform/mac-wk2/plugins/script-object-access-fails-during-slow-initialization.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@128088 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/plugins/script-object-access-fails-during-slow-initialization-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac-wk2/plugins/script-object-access-fails-during-slow-initialization.html [new file with mode: 0644]
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.h
Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.h
Source/WebKit2/WebProcess/Plugins/Plugin.h
Source/WebKit2/WebProcess/Plugins/PluginProxy.cpp
Source/WebKit2/WebProcess/Plugins/PluginProxy.h
Source/WebKit2/WebProcess/Plugins/PluginView.cpp
Tools/ChangeLog
Tools/DumpRenderTree/TestNetscapePlugIn/Tests/SlowNPPNew.cpp

index 51da554..0dc26da 100644 (file)
@@ -1,3 +1,13 @@
+2012-09-10  Brady Eidson  <beidson@apple.com>
+
+        Javascript in foreground tabs should not wait synchronously for plug-ins to load
+        <rdar://problem/12067415> and https://bugs.webkit.org/show_bug.cgi?id=96167
+
+        Reviewed by Geoff Garen.
+
+        * platform/mac-wk2/plugins/script-object-access-fails-during-slow-initialization-expected.txt: Added.
+        * platform/mac-wk2/plugins/script-object-access-fails-during-slow-initialization.html: Added.
+
 2012-09-07  Jer Noble  <jer.noble@apple.com>
 
         <audio> and <video> should send Do Not Track when appropriate
diff --git a/LayoutTests/platform/mac-wk2/plugins/script-object-access-fails-during-slow-initialization-expected.txt b/LayoutTests/platform/mac-wk2/plugins/script-object-access-fails-during-slow-initialization-expected.txt
new file mode 100644 (file)
index 0000000..b102fd3
--- /dev/null
@@ -0,0 +1,8 @@
+
+Tests that accessing the script object of a plug-in that hasn't been initialized fails instead of waiting for initialization to finish.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Accessing the property took less than 550ms, meaning the plug-in had not finished initializing after our call to the plug-in script object returned.
+
diff --git a/LayoutTests/platform/mac-wk2/plugins/script-object-access-fails-during-slow-initialization.html b/LayoutTests/platform/mac-wk2/plugins/script-object-access-fails-during-slow-initialization.html
new file mode 100644 (file)
index 0000000..08e410c
--- /dev/null
@@ -0,0 +1,47 @@
+<head>
+<script src="../../../fast/js/resources/js-test-pre.js"></script>
+<script>
+
+var startTime = new Date;
+
+if (window.testRunner) {
+    testRunner.overridePreference("WebKit2AsynchronousPluginInitializationEnabled", "1");
+    testRunner.overridePreference("WebKit2AsynchronousPluginInitializationEnabledForAllPlugins", "1");
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+function runTest() 
+{
+    if (!window.testRunner) {
+        debug("This test can only run from within DumpRenderTree because it requires test runner internals.\n");
+        return;
+    }
+    
+    var pluginElement = document.getElementById("TestElement");
+    var testProperty = pluginElement.fooBar;
+    
+    if (testProperty)
+        testFailed("testProperty should not have returned anything, but it returned " + testProperty);
+
+    var endTime = new Date;
+    if (endTime - startTime > 549)
+        testFailed("This test took over 549ms meaning the plug-in with a 550ms startup delay was actually initialized.  It never should've been initialized.");
+    else
+        testPassed("Accessing the property took less than 550ms, meaning the plug-in had not finished initializing after our call to the plug-in script object returned.");
+
+    testRunner.notifyDone();
+}
+</script>
+
+</head>
+<body onload="setTimeout('runTest()', 0)">
+<embed id="TestElement" type="application/x-webkit-test-netscape" test="slow-npp-new"></embed>
+<p id="description"></p>
+<div id="console"></div>
+</body>
+
+<script>
+description("Tests that accessing the script object of a plug-in that hasn't been initialized fails instead of waiting for initialization to finish.");
+var unused = document.body.offsetTop;
+</script>
index 5c68484..5f47aa4 100644 (file)
@@ -1,3 +1,32 @@
+2012-09-10  Brady Eidson  <beidson@apple.com>
+
+        Javascript in foreground tabs should not wait synchronously for plug-ins to load
+        <rdar://problem/12067415> and https://bugs.webkit.org/show_bug.cgi?id=96167
+
+        Reviewed by Geoff Garen.
+
+        Synchronously waiting for initialization to complete when javascript accesses the plug-in script object severely
+        reduces the effectiveness of having an asynchronous NPP_New.
+
+        Such as with background tabs that have never been viewed, we already have cases where JS calls into the plug-in 
+        element fail because we haven't bothered to initialize the plug-in.
+
+        We get a huge win by expanding that to foreground tabs that simply haven't finished initializing their plug-ins.
+
+        * WebProcess/Plugins/PluginView.cpp:
+        (WebKit::PluginView::scriptObject): If initialization is not complete just return 0 - They can get at the script object later.
+
+        Remove the notion of "wait for asynchronous initialization" altogether:
+        * WebProcess/Plugins/Netscape/NetscapePlugin.h:
+        (NetscapePlugin):
+        * WebProcess/Plugins/PDF/BuiltInPDFView.h:
+        (BuiltInPDFView):
+        * WebProcess/Plugins/Plugin.h:
+        (Plugin):
+        * WebProcess/Plugins/PluginProxy.cpp:
+        * WebProcess/Plugins/PluginProxy.h:
+        (PluginProxy):
+
 2012-09-07  Jer Noble  <jer.noble@apple.com>
 
         <audio> and <video> should send Do Not Track when appropriate
index 5f93c57..f0d948b 100644 (file)
@@ -55,7 +55,6 @@ public:
     static PassRefPtr<NetscapePlugin> fromNPP(NPP);
 
     // In-process NetscapePlugins don't support asynchronous initialization.
-    virtual void waitForAsynchronousInitialization() { }
     virtual bool isBeingAsynchronouslyInitialized() const { return false; }
 
 #if PLATFORM(MAC)
index 9ccd804..cdb5cf0 100644 (file)
@@ -51,7 +51,6 @@ public:
     static WebCore::PluginInfo pluginInfo();
 
     // In-process PDFViews don't support asynchronous initialization.
-    virtual void waitForAsynchronousInitialization() { }
     virtual bool isBeingAsynchronouslyInitialized() const { return false; }
 
 private:
index b91174f..91d234f 100644 (file)
@@ -83,8 +83,6 @@ public:
     // Sets the active plug-in controller and initializes the plug-in.
     bool initialize(PluginController*, const Parameters&);
 
-    // Forces synchronous initialization of a plugin previously initialized asynchronously.
-    virtual void waitForAsynchronousInitialization() = 0;
     virtual bool isBeingAsynchronouslyInitialized() const = 0;
 
     // Destroys the plug-in.
index e380eef..d3eb28a 100644 (file)
@@ -123,14 +123,6 @@ bool PluginProxy::canInitializeAsynchronously() const
     return controller()->asynchronousPluginInitializationEnabled() && (m_connection->supportsAsynchronousPluginInitialization() || controller()->asynchronousPluginInitializationEnabledForAllPlugins());
 }
 
-void PluginProxy::waitForAsynchronousInitialization()
-{
-    ASSERT(!m_isStarted);
-    ASSERT(m_waitingOnAsynchronousInitialization);
-
-    initializeSynchronously();
-}
-
 bool PluginProxy::initializeSynchronously()
 {
     ASSERT(m_pendingPluginCreationParameters);
index c0ca116..c3183bb 100644 (file)
@@ -71,7 +71,6 @@ private:
     virtual bool initialize(const Parameters&);
     bool initializeSynchronously();
 
-    virtual void waitForAsynchronousInitialization();
     virtual void destroy();
     virtual void paint(WebCore::GraphicsContext*, const WebCore::IntRect& dirtyRect);
     virtual PassRefPtr<ShareableBitmap> snapshot();
index 0a0c6b6..8f50d87 100644 (file)
@@ -550,19 +550,8 @@ JSObject* PluginView::scriptObject(JSGlobalObject* globalObject)
     if (m_isWaitingForSynchronousInitialization)
         return 0;
 
-    // The plug-in can be null here if it failed to initialize previously.
-    if (!m_plugin)
-        return 0;
-
-    // If the plug-in exists but is not initialized then we're still initializing asynchronously.
-    // We need to wait here until initialization has either succeeded or failed.
-    if (m_plugin->isBeingAsynchronouslyInitialized()) {
-        m_isWaitingForSynchronousInitialization = true;
-        m_plugin->waitForAsynchronousInitialization();
-        m_isWaitingForSynchronousInitialization = false;
-    }
-
-    // The plug-in can be null here if it still failed to initialize.
+    // We might not have started initialization of the plug-in yet, the plug-in might be in the middle
+    // of being initializing asynchronously, or initialization might have previously failed.
     if (!m_isInitialized || !m_plugin)
         return 0;
 
index 6324a26..c9fedbe 100644 (file)
@@ -1,3 +1,21 @@
+2012-09-10  Brady Eidson  <beidson@apple.com>
+
+        Javascript in foreground tabs should not wait synchronously for plug-ins to load
+        <rdar://problem/12067415> and https://bugs.webkit.org/show_bug.cgi?id=96167
+
+        Reviewed by Geoff Garen.
+
+        Enhance the "Slow NPP_New" plug-in to also be able to return properties to javascript.
+
+        * DumpRenderTree/TestNetscapePlugIn/Tests/SlowNPPNew.cpp:
+        (PluginObject):
+        (SlowNPPNew::PluginObject::PluginObject):
+        (SlowNPPNew::PluginObject::~PluginObject):
+        (SlowNPPNew::PluginObject::hasProperty):
+        (SlowNPPNew::PluginObject::getProperty):
+        (SlowNPPNew::NPP_GetValue):
+        (SlowNPPNew):
+
 2012-09-10  Tim Horton  <timothy_horton@apple.com>
 
         WKTR often reports an unresponsive WebProcess on Mac bots
index bd2a76a..8c80d55 100644 (file)
@@ -37,7 +37,46 @@ public:
     }
     
 private:
+    class PluginObject : public Object<PluginObject> {
+    public:
+        PluginObject()
+        {
+        }
+
+        ~PluginObject()
+        {
+        }
+
+        bool hasProperty(NPIdentifier propertyName)
+        {
+            return true;
+        }
+
+        bool getProperty(NPIdentifier propertyName, NPVariant* result)
+        {
+            static const char* message = "My name is ";
+            char* propertyString = pluginTest()->NPN_UTF8FromIdentifier(propertyName);
+            
+            int bufferLength = strlen(propertyString) + strlen(message) + 1;
+            char* resultBuffer = static_cast<char*>(pluginTest()->NPN_MemAlloc(bufferLength));
+            snprintf(resultBuffer, bufferLength, "%s%s", message, propertyString);
+            
+            STRINGZ_TO_NPVARIANT(resultBuffer, *result);
+
+            return true;
+        }
+    };
     
+    virtual NPError NPP_GetValue(NPPVariable variable, void *value)
+    {
+        if (variable != NPPVpluginScriptableNPObject)
+            return NPERR_GENERIC_ERROR;
+        
+        *(NPObject**)value = PluginObject::create(this);
+        
+        return NPERR_NO_ERROR;
+    }
+
     virtual NPError NPP_New(NPMIMEType pluginType, uint16_t mode, int16_t argc, char* argn[], char* argv[], NPSavedData *saved)
     {
         usleep(550000);