Title: [143650] trunk/Source
Revision
143650
Author
d...@apple.com
Date
2013-02-21 13:53:01 -0800 (Thu, 21 Feb 2013)

Log Message

Plug-in snapshotting code always accepts first snapshot
https://bugs.webkit.org/show_bug.cgi?id=110495

Reviewed by Tim Horton.

When we detect a plugin that can be snapshotted we start capturing
images until we find one that we believe isn't blank, or we timeout.
I introduced a regression recently where we swap renderers as
soon as the snapshot arrives, whether or not is is blank.

The fix was to have the embedder (currently only WK2) be the one
who tells the HTMLPlugInElement to start displaying snapshots.

Source/WebCore:

* html/HTMLPlugInElement.h:
(WebCore::HTMLPlugInElement::setDisplayState): Make this a virtual function.
* html/HTMLPlugInImageElement.cpp:
(WebCore::HTMLPlugInImageElement::setDisplayState): Override to call the
    base class version, but swap renderers if we should move to the snapshot.
(WebCore::HTMLPlugInImageElement::updateSnapshot): Don't swap renderers here any more.
* html/HTMLPlugInImageElement.h: New virtual version of setDisplayState.

Source/WebKit2:

* WebProcess/Plugins/PluginView.cpp:
(WebKit::PluginView::pluginSnapshotTimerFired): When we find a
snapshot that we like, tell the HTMLPlugInElement to move to
the snapshot view.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (143649 => 143650)


--- trunk/Source/WebCore/ChangeLog	2013-02-21 21:46:26 UTC (rev 143649)
+++ trunk/Source/WebCore/ChangeLog	2013-02-21 21:53:01 UTC (rev 143650)
@@ -1,3 +1,29 @@
+2013-02-21  Dean Jackson  <d...@apple.com>
+
+        Plug-in snapshotting code always accepts first snapshot
+        https://bugs.webkit.org/show_bug.cgi?id=110495
+
+        Reviewed by Tim Horton.
+
+        When we detect a plugin that can be snapshotted we start capturing
+        images until we find one that we believe isn't blank, or we timeout.
+        I introduced a regression recently where we swap renderers as
+        soon as the snapshot arrives, whether or not is is blank.
+
+        The fix was to have the embedder (currently only WK2) be the one
+        who tells the HTMLPlugInElement to start displaying snapshots.
+
+        I also reduced the number of snapshot attempts we will make before
+        giving up. We don't want to sit around for 66 seconds displaying nothing.
+
+        * html/HTMLPlugInElement.h:
+        (WebCore::HTMLPlugInElement::setDisplayState): Make this a virtual function.
+        * html/HTMLPlugInImageElement.cpp:
+        (WebCore::HTMLPlugInImageElement::setDisplayState): Override to call the
+            base class version, but swap renderers if we should move to the snapshot.
+        (WebCore::HTMLPlugInImageElement::updateSnapshot): Don't swap renderers here any more.
+        * html/HTMLPlugInImageElement.h: New virtual version of setDisplayState.
+
 2013-02-21  Tony Gentilcore  <to...@chromium.org>
 
         Tune BackgroundHTMLParser's pendingTokenLimit based on a benchmark

Modified: trunk/Source/WebCore/html/HTMLPlugInElement.h (143649 => 143650)


--- trunk/Source/WebCore/html/HTMLPlugInElement.h	2013-02-21 21:46:26 UTC (rev 143649)
+++ trunk/Source/WebCore/html/HTMLPlugInElement.h	2013-02-21 21:53:01 UTC (rev 143650)
@@ -55,7 +55,7 @@
         Playing
     };
     DisplayState displayState() const { return m_displayState; }
-    void setDisplayState(DisplayState state) { m_displayState = state; }
+    virtual void setDisplayState(DisplayState state) { m_displayState = state; }
     virtual void updateSnapshot(PassRefPtr<Image>) { }
     virtual void dispatchPendingMouseClick() { }
 

Modified: trunk/Source/WebCore/html/HTMLPlugInImageElement.cpp (143649 => 143650)


--- trunk/Source/WebCore/html/HTMLPlugInImageElement.cpp	2013-02-21 21:46:26 UTC (rev 143649)
+++ trunk/Source/WebCore/html/HTMLPlugInImageElement.cpp	2013-02-21 21:53:01 UTC (rev 143650)
@@ -86,6 +86,13 @@
         document()->unregisterForPageCacheSuspensionCallbacks(this);
 }
 
+void HTMLPlugInImageElement::setDisplayState(DisplayState state)
+{
+    HTMLPlugInElement::setDisplayState(state);
+    if (displayState() == DisplayingSnapshot)
+        m_swapRendererTimer.startOneShot(0);
+}
+
 RenderEmbeddedObject* HTMLPlugInImageElement::renderEmbeddedObject() const
 {
     // HTMLObjectElement and HTMLEmbedElement may return arbitrary renderers
@@ -287,9 +294,6 @@
         toRenderSnapshottedPlugIn(renderer())->updateSnapshot(image);
         return;
     }
-
-    setDisplayState(DisplayingSnapshot);
-    m_swapRendererTimer.startOneShot(0);
 }
 
 static AtomicString classNameForShadowRootSize(const IntSize& viewContentsSize, const Node* node)

Modified: trunk/Source/WebCore/html/HTMLPlugInImageElement.h (143649 => 143650)


--- trunk/Source/WebCore/html/HTMLPlugInImageElement.h	2013-02-21 21:46:26 UTC (rev 143649)
+++ trunk/Source/WebCore/html/HTMLPlugInImageElement.h	2013-02-21 21:53:01 UTC (rev 143650)
@@ -52,6 +52,8 @@
 
     RenderEmbeddedObject* renderEmbeddedObject() const;
 
+    virtual void setDisplayState(DisplayState) OVERRIDE;
+
     virtual void updateWidget(PluginCreationOption) = 0;
 
     const String& serviceType() const { return m_serviceType; }

Modified: trunk/Source/WebKit2/ChangeLog (143649 => 143650)


--- trunk/Source/WebKit2/ChangeLog	2013-02-21 21:46:26 UTC (rev 143649)
+++ trunk/Source/WebKit2/ChangeLog	2013-02-21 21:53:01 UTC (rev 143650)
@@ -1,3 +1,23 @@
+2013-02-21  Dean Jackson  <d...@apple.com>
+
+        Plug-in snapshotting code always accepts first snapshot
+        https://bugs.webkit.org/show_bug.cgi?id=110495
+
+        Reviewed by Tim Horton.
+
+        When we detect a plugin that can be snapshotted we start capturing
+        images until we find one that we believe isn't blank, or we timeout.
+        I introduced a regression recently where we swap renderers as
+        soon as the snapshot arrives, whether or not is is blank.
+
+        The fix was to have the embedder (currently only WK2) be the one
+        who tells the HTMLPlugInElement to start displaying snapshots.
+
+        * WebProcess/Plugins/PluginView.cpp:
+        (WebKit::PluginView::pluginSnapshotTimerFired): When we find a
+        snapshot that we like, tell the HTMLPlugInElement to move to
+        the snapshot view.
+
 2013-02-21  Carlos Garcia Campos  <cgar...@igalia.com>
 
         Unreviewed. Fix make distcheck.

Modified: trunk/Source/WebKit2/WebProcess/Plugins/PluginView.cpp (143649 => 143650)


--- trunk/Source/WebKit2/WebProcess/Plugins/PluginView.cpp	2013-02-21 21:46:26 UTC (rev 143649)
+++ trunk/Source/WebKit2/WebProcess/Plugins/PluginView.cpp	2013-02-21 21:53:01 UTC (rev 143650)
@@ -71,7 +71,7 @@
 
 // This simulated mouse click delay in HTMLPlugInImageElement.cpp should generally be the same or shorter than this delay.
 static const double pluginSnapshotTimerDelay = 1.1;
-static const unsigned maximumSnapshotRetries = 60;
+static const unsigned maximumSnapshotRetries = 4;
 
 class PluginView::URLRequest : public RefCounted<URLRequest> {
 public:
@@ -1606,8 +1606,7 @@
     }
 #endif
 
-    destroyPluginAndReset();
-    m_plugin = 0;
+    m_pluginElement->setDisplayState(HTMLPlugInElement::DisplayingSnapshot);
 }
 
 bool PluginView::shouldAlwaysAutoStart() const
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to