[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc

eric at webkit.org eric at webkit.org
Wed Dec 22 12:58:16 UTC 2010


The following commit has been merged in the debian/experimental branch:
commit 4ffb1fe5923609044bf1ff2448f256224cd5f313
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Fri Sep 3 04:28:12 2010 +0000

    2010-09-02  Eric Seidel  <eric at webkit.org>
    
            Reviewed by Dimitri Glazkov.
    
            Move updateWidget implementations into the DOM
            https://bugs.webkit.org/show_bug.cgi?id=45058
    
            Unfortunately it's not yet possible to share an updateWidget
            implementation between <embed> and <object>.  That would amount to
            (positive) functional changes for <embed> which I'd will separate
            into a later patch.  I will also have to untangle <object>'s <param>
            walk from its url/serviceType calculations.
    
            However after this patch it's slap-you-across-the-face obvious that
            RenderEmbeddedObject::updateWidget was the wrong place for this code.
            RenderEmbeddedObject::updateWidget still exists, but only as a
            pseudo-virtual-method dispatcher.  Unless we add updateWidget() to
            HTMLElement, we won't be able to use real virtual dispatch.
    
            I may need to consider making "having a widget" a has-a relationship
            with some sort of WidgetContainer object which Media and Plugin can
            share.  Or its also possible that Media's use of the widget code here
            is just wrong.  Certainly now that updateWidget was moved into HTMLMediaElement
            it becomes obvious that HTMLMediaElement has duplicate code for updating widgets.
    
            No functional changes, thus no tests.
    
            * html/HTMLEmbedElement.cpp:
            (WebCore::HTMLEmbedElement::updateWidget):
            * html/HTMLEmbedElement.h:
            * html/HTMLMediaElement.cpp:
            (WebCore::HTMLMediaElement::updateWidget):
            * html/HTMLMediaElement.h:
            * html/HTMLObjectElement.cpp:
            (WebCore::HTMLObjectElement::updateWidget):
            * html/HTMLObjectElement.h:
            (WebCore::HTMLObjectElement::useFallbackContent):
            * html/HTMLPlugInImageElement.h:
            (WebCore::HTMLPlugInImageElement::needsWidgetUpdate):
            (WebCore::HTMLPlugInImageElement::setNeedsWidgetUpdate):
            * loader/FrameLoader.cpp:
            * rendering/RenderEmbeddedObject.cpp:
            (WebCore::RenderEmbeddedObject::updateWidget):
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@66710 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 4c5c878..2c3dc74 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -2,6 +2,50 @@
 
         Reviewed by Dimitri Glazkov.
 
+        Move updateWidget implementations into the DOM
+        https://bugs.webkit.org/show_bug.cgi?id=45058
+
+        Unfortunately it's not yet possible to share an updateWidget
+        implementation between <embed> and <object>.  That would amount to
+        (positive) functional changes for <embed> which I'd will separate
+        into a later patch.  I will also have to untangle <object>'s <param>
+        walk from its url/serviceType calculations.
+
+        However after this patch it's slap-you-across-the-face obvious that
+        RenderEmbeddedObject::updateWidget was the wrong place for this code.
+        RenderEmbeddedObject::updateWidget still exists, but only as a
+        pseudo-virtual-method dispatcher.  Unless we add updateWidget() to
+        HTMLElement, we won't be able to use real virtual dispatch.
+
+        I may need to consider making "having a widget" a has-a relationship
+        with some sort of WidgetContainer object which Media and Plugin can
+        share.  Or its also possible that Media's use of the widget code here
+        is just wrong.  Certainly now that updateWidget was moved into HTMLMediaElement
+        it becomes obvious that HTMLMediaElement has duplicate code for updating widgets.
+
+        No functional changes, thus no tests.
+
+        * html/HTMLEmbedElement.cpp:
+        (WebCore::HTMLEmbedElement::updateWidget):
+        * html/HTMLEmbedElement.h:
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::updateWidget):
+        * html/HTMLMediaElement.h:
+        * html/HTMLObjectElement.cpp:
+        (WebCore::HTMLObjectElement::updateWidget):
+        * html/HTMLObjectElement.h:
+        (WebCore::HTMLObjectElement::useFallbackContent):
+        * html/HTMLPlugInImageElement.h:
+        (WebCore::HTMLPlugInImageElement::needsWidgetUpdate):
+        (WebCore::HTMLPlugInImageElement::setNeedsWidgetUpdate):
+        * loader/FrameLoader.cpp:
+        * rendering/RenderEmbeddedObject.cpp:
+        (WebCore::RenderEmbeddedObject::updateWidget):
+
+2010-09-02  Eric Seidel  <eric at webkit.org>
+
+        Reviewed by Dimitri Glazkov.
+
         Move more code from RenderEmbeddedObject into the DOM
         https://bugs.webkit.org/show_bug.cgi?id=45055
 
diff --git a/WebCore/html/HTMLEmbedElement.cpp b/WebCore/html/HTMLEmbedElement.cpp
index 74b9715..0ab00ec 100644
--- a/WebCore/html/HTMLEmbedElement.cpp
+++ b/WebCore/html/HTMLEmbedElement.cpp
@@ -133,6 +133,38 @@ void HTMLEmbedElement::parametersForPlugin(Vector<String>& paramNames, Vector<St
     }
 }
 
+// FIXME: This should be unified with HTMLObjectElement::updateWidget and
+// moved down into HTMLPluginImageElement.cpp
+void HTMLEmbedElement::updateWidget(bool onlyCreateNonNetscapePlugins)
+{
+    // FIXME: We should ASSERT(needsWidgetUpdate()), but currently
+    // FrameView::updateWidget() calls updateWidget(false) without checking if
+    // the widget actually needs updating!
+    setNeedsWidgetUpdate(false);
+
+    if (m_url.isEmpty() && m_serviceType.isEmpty())
+        return;
+
+    // Note these pass m_url and m_serviceType to allow better code sharing with
+    // <object> which modifies url and serviceType before calling these.
+    if (!allowedToLoadFrameURL(m_url))
+        return;
+    if (onlyCreateNonNetscapePlugins && wouldLoadAsNetscapePlugin(m_url, m_serviceType))
+        return;
+
+    // FIXME: These should be joined into a PluginParameters class.
+    Vector<String> paramNames;
+    Vector<String> paramValues;
+    parametersForPlugin(paramNames, paramValues);
+
+    if (!dispatchBeforeLoadEvent(m_url))
+        return;
+
+    SubframeLoader* loader = document()->frame()->loader()->subframeLoader();
+    // FIXME: beforeLoad could have detached the renderer!  Just like in the <object> case above.
+    loader->requestObject(this, m_url, getAttribute(nameAttr), m_serviceType, paramNames, paramValues);
+}
+
 bool HTMLEmbedElement::rendererIsNeeded(RenderStyle* style)
 {
     if (isImageType())
diff --git a/WebCore/html/HTMLEmbedElement.h b/WebCore/html/HTMLEmbedElement.h
index d4a4d7f..0fb7f50 100644
--- a/WebCore/html/HTMLEmbedElement.h
+++ b/WebCore/html/HTMLEmbedElement.h
@@ -31,7 +31,7 @@ class HTMLEmbedElement : public HTMLPlugInImageElement {
 public:
     static PassRefPtr<HTMLEmbedElement> create(const QualifiedName&, Document*, bool createdByParser);
 
-    void parametersForPlugin(Vector<String>& paramNames, Vector<String>& paramValues);
+    void updateWidget(bool onlyCreateNonNetscapePlugins);
 
 private:
     HTMLEmbedElement(const QualifiedName&, Document*, bool createdByParser);
@@ -50,6 +50,8 @@ private:
     virtual RenderWidget* renderWidgetForJSBindings() const;
 
     virtual void addSubresourceAttributeURLs(ListHashSet<KURL>&) const;
+
+    void parametersForPlugin(Vector<String>& paramNames, Vector<String>& paramValues);
 };
 
 }
diff --git a/WebCore/html/HTMLMediaElement.cpp b/WebCore/html/HTMLMediaElement.cpp
index 8c858c5..4621096 100644
--- a/WebCore/html/HTMLMediaElement.cpp
+++ b/WebCore/html/HTMLMediaElement.cpp
@@ -2010,6 +2010,20 @@ void HTMLMediaElement::createMediaPlayerProxy()
     if (m_proxyWidget)
         m_needWidgetUpdate = false;
 }
+
+void HTMLMediaElement::updateWidget(bool)
+{
+    mediaElement->setNeedWidgetUpdate(false);
+
+    Vector<String> paramNames;
+    Vector<String> paramValues;
+    KURL kurl;
+    
+    mediaElement->getPluginProxyParams(kurl, paramNames, paramValues);
+    SubframeLoader* loader = document()->frame()->loader()->subframeLoader();
+    loader->loadMediaPlayerProxyPlugin(mediaElement, kurl, paramNames, paramValues);
+}
+
 #endif // ENABLE(PLUGIN_PROXY_FOR_VIDEO)
 
 void HTMLMediaElement::enterFullscreen()
diff --git a/WebCore/html/HTMLMediaElement.h b/WebCore/html/HTMLMediaElement.h
index 3895fe3..b595604 100644
--- a/WebCore/html/HTMLMediaElement.h
+++ b/WebCore/html/HTMLMediaElement.h
@@ -153,6 +153,7 @@ public:
     void getPluginProxyParams(KURL& url, Vector<String>& names, Vector<String>& values);
     virtual void finishParsingChildren();
     void createMediaPlayerProxy();
+    void updateWidget(bool onlyCreateNonNetscapePlugins);
 #endif
 
     bool hasSingleSecurityOrigin() const { return !m_player || m_player->hasSingleSecurityOrigin(); }
diff --git a/WebCore/html/HTMLObjectElement.cpp b/WebCore/html/HTMLObjectElement.cpp
index 2b879ba..c970293 100644
--- a/WebCore/html/HTMLObjectElement.cpp
+++ b/WebCore/html/HTMLObjectElement.cpp
@@ -236,6 +236,52 @@ bool HTMLObjectElement::hasFallbackContent() const
     return false;
 }
 
+// FIXME: This should be unified with HTMLEmbedElement::updateWidget and
+// moved down into HTMLPluginImageElement.cpp
+void HTMLObjectElement::updateWidget(bool onlyCreateNonNetscapePlugins)
+{
+    // FIXME: We should ASSERT(needsWidgetUpdate()), but currently
+    // FrameView::updateWidget() calls updateWidget(false) without checking if
+    // the widget actually needs updating!
+    setNeedsWidgetUpdate(false);
+    // FIXME: This should ASSERT isFinishedParsingChildren() instead.
+    if (!isFinishedParsingChildren())
+        return;
+
+    String url = this->url();
+    String serviceType = this->serviceType();
+
+    // FIXME: These should be joined into a PluginParameters class.
+    Vector<String> paramNames;
+    Vector<String> paramValues;
+    parametersForPlugin(paramNames, paramValues, url, serviceType);
+
+    // Note: url is modified above by parametersForPlugin.
+    if (!allowedToLoadFrameURL(url))
+        return;
+
+    bool fallbackContent = hasFallbackContent();
+    renderEmbeddedObject()->setHasFallbackContent(fallbackContent);
+
+    if (onlyCreateNonNetscapePlugins && wouldLoadAsNetscapePlugin(url, serviceType))
+        return;
+
+    bool beforeLoadAllowedLoad = dispatchBeforeLoadEvent(url);
+
+    // beforeload events can modify the DOM, potentially causing
+    // RenderWidget::destroy() to be called.  Ensure we haven't been
+    // destroyed before continuing.
+    // FIXME: Should this render fallback content?
+    if (!renderer())
+        return;
+
+    SubframeLoader* loader = document()->frame()->loader()->subframeLoader();
+    bool success = beforeLoadAllowedLoad && loader->requestObject(this, url, getAttribute(nameAttr), serviceType, paramNames, paramValues);
+
+    if (!success && fallbackContent)
+        renderFallbackContent();
+}
+
 bool HTMLObjectElement::rendererIsNeeded(RenderStyle* style)
 {
     // FIXME: This check should not be needed, detached documents never render!
diff --git a/WebCore/html/HTMLObjectElement.h b/WebCore/html/HTMLObjectElement.h
index 3ff9218..36c3c4b 100644
--- a/WebCore/html/HTMLObjectElement.h
+++ b/WebCore/html/HTMLObjectElement.h
@@ -31,20 +31,16 @@ class HTMLObjectElement : public HTMLPlugInImageElement {
 public:
     static PassRefPtr<HTMLObjectElement> create(const QualifiedName&, Document*, bool createdByParser);
 
-    void renderFallbackContent();
-
     bool isDocNamedItem() const { return m_docNamedItem; }
 
     const String& classId() const { return m_classId; }
 
     bool containsJavaApplet() const;
 
-    bool hasFallbackContent() const;
-    virtual bool useFallbackContent() const { return m_useFallbackContent; }
+    void updateWidget(bool onlyCreateNonNetscapePlugins);
 
-    // FIXME: This function should not deal with url or serviceType
-    // so that we can better share code between <object> and <embed>.
-    void parametersForPlugin(Vector<String>& paramNames, Vector<String>& paramValues, String& url, String& serviceType);
+    virtual bool useFallbackContent() const { return m_useFallbackContent; }
+    void renderFallbackContent();
 
 private:
     HTMLObjectElement(const QualifiedName&, Document*, bool createdByParser);
@@ -66,6 +62,12 @@ private:
 
     void updateDocNamedItem();
 
+    bool hasFallbackContent() const;
+    
+    // FIXME: This function should not deal with url or serviceType
+    // so that we can better share code between <object> and <embed>.
+    void parametersForPlugin(Vector<String>& paramNames, Vector<String>& paramValues, String& url, String& serviceType);
+
     AtomicString m_id;
     String m_classId;
     bool m_docNamedItem : 1;
diff --git a/WebCore/html/HTMLPlugInImageElement.h b/WebCore/html/HTMLPlugInImageElement.h
index 814aaf0..78bdeb3 100644
--- a/WebCore/html/HTMLPlugInImageElement.h
+++ b/WebCore/html/HTMLPlugInImageElement.h
@@ -35,12 +35,6 @@ public:
     const String& serviceType() const { return m_serviceType; }
     const String& url() const { return m_url; }
 
-    // These can all move to be protected once updateWidget is moved out of RenderEmbeddedObject.cpp
-    bool needsWidgetUpdate() const { return m_needsWidgetUpdate; }
-    void setNeedsWidgetUpdate(bool needsWidgetUpdate) { m_needsWidgetUpdate = needsWidgetUpdate; }
-    bool allowedToLoadFrameURL(const String& url);
-    bool wouldLoadAsNetscapePlugin(const String& url, const String& serviceType);
-
     RenderEmbeddedObject* renderEmbeddedObject() const;
 
 protected:
@@ -56,6 +50,12 @@ protected:
     virtual void attach();
     virtual void detach();
 
+    bool needsWidgetUpdate() const { return m_needsWidgetUpdate; }
+    void setNeedsWidgetUpdate(bool needsWidgetUpdate) { m_needsWidgetUpdate = needsWidgetUpdate; }
+
+    bool allowedToLoadFrameURL(const String& url);
+    bool wouldLoadAsNetscapePlugin(const String& url, const String& serviceType);
+
 private:
     virtual bool canLazyAttach() { return false; }
     virtual RenderObject* createRenderer(RenderArena*, RenderStyle*);
diff --git a/WebCore/loader/FrameLoader.cpp b/WebCore/loader/FrameLoader.cpp
index ccbccc9..eeb5283 100644
--- a/WebCore/loader/FrameLoader.cpp
+++ b/WebCore/loader/FrameLoader.cpp
@@ -1042,6 +1042,7 @@ void FrameLoader::setOpener(Frame* opener)
     }
 }
 
+// FIXME: This does not belong in FrameLoader!
 void FrameLoader::handleFallbackContent()
 {
     HTMLFrameOwnerElement* owner = m_frame->ownerElement();
diff --git a/WebCore/rendering/RenderEmbeddedObject.cpp b/WebCore/rendering/RenderEmbeddedObject.cpp
index 9f8acfd..43c8d22 100644
--- a/WebCore/rendering/RenderEmbeddedObject.cpp
+++ b/WebCore/rendering/RenderEmbeddedObject.cpp
@@ -101,95 +101,8 @@ bool RenderEmbeddedObject::allowsAcceleratedCompositing() const
     return widget() && widget()->isPluginViewBase() && static_cast<PluginViewBase*>(widget())->platformLayer();
 }
 #endif
-    
-// FIXME: This belongs on HTMLObjectElement, HTMLPluginElement or HTMLFrameOwnerElement.
-static void updateWidgetForObjectElement(HTMLObjectElement* objectElement, bool onlyCreateNonNetscapePlugins)
-{
-    // FIXME: We should ASSERT(objectElement->needsWidgetUpdate()), but currently
-    // FrameView::updateWidget() calls updateWidget(false) without checking if
-    // the widget actually needs updating!
-    objectElement->setNeedsWidgetUpdate(false);
-    // FIXME: This should ASSERT isFinishedParsingChildren() instead.
-    if (!objectElement->isFinishedParsingChildren())
-        return;
-
-    String url = objectElement->url();
-    String serviceType = objectElement->serviceType();
-
-    Vector<String> paramNames;
-    Vector<String> paramValues;
-    objectElement->parametersForPlugin(paramNames, paramValues, url, serviceType);
-
-    // Note: url is modified above by parametersForPlugin.
-    if (!objectElement->allowedToLoadFrameURL(url))
-        return;
-
-    bool fallbackContent = objectElement->hasFallbackContent();
-    objectElement->renderEmbeddedObject()->setHasFallbackContent(fallbackContent);
-
-    if (onlyCreateNonNetscapePlugins && objectElement->wouldLoadAsNetscapePlugin(url, serviceType))
-        return;
-
-    bool beforeLoadAllowedLoad = objectElement->dispatchBeforeLoadEvent(url);
-
-    // beforeload events can modify the DOM, potentially causing
-    // RenderWidget::destroy() to be called.  Ensure we haven't been
-    // destroyed before continuing.
-    // FIXME: Should this render fallback content?
-    if (!objectElement->renderer())
-        return;
-
-    SubframeLoader* loader = objectElement->document()->frame()->loader()->subframeLoader();
-    bool success = beforeLoadAllowedLoad && loader->requestObject(objectElement, url, objectElement->getAttribute(nameAttr), serviceType, paramNames, paramValues);
-
-    if (!success && fallbackContent)
-        objectElement->renderFallbackContent();
-}
-
-// FIXME: This belongs on HTMLEmbedElement, HTMLPluginElement or HTMLFrameOwnerElement.
-static void updateWidgetForEmbedElement(HTMLEmbedElement* embedElement, bool onlyCreateNonNetscapePlugins)
-{
-    // FIXME: We should ASSERT(embedElement->needsWidgetUpdate()), but currently
-    // FrameView::updateWidget() calls updateWidget(false) without checking if
-    // the widget actually needs updating!
-    embedElement->setNeedsWidgetUpdate(false);
-
-    String url = embedElement->url();
-    String serviceType = embedElement->serviceType();
-    if (url.isEmpty() && serviceType.isEmpty())
-        return;
-    if (!embedElement->allowedToLoadFrameURL(url))
-        return;
-
-    if (onlyCreateNonNetscapePlugins && embedElement->wouldLoadAsNetscapePlugin(url, serviceType))
-        return;
-
-    Vector<String> paramNames;
-    Vector<String> paramValues;
-    embedElement->parametersForPlugin(paramNames, paramValues);
-
-    if (!embedElement->dispatchBeforeLoadEvent(url))
-        return;
-
-    SubframeLoader* loader = embedElement->document()->frame()->loader()->subframeLoader();
-    // FIXME: beforeLoad could have detached the renderer!  Just like in the <object> case above.
-    loader->requestObject(embedElement, url, embedElement->getAttribute(nameAttr), serviceType, paramNames, paramValues);
-}
-
-#if ENABLE(PLUGIN_PROXY_FOR_VIDEO)
-// FIXME: This belongs on HTMLMediaElement.
-static void updateWidgetForMediaElement(HTMLMediaElement* mediaElement, bool ignored)
-{
-    Vector<String> paramNames;
-    Vector<String> paramValues;
-    KURL kurl;
-
-    mediaElement->getPluginProxyParams(kurl, paramNames, paramValues);
-    mediaElement->setNeedWidgetUpdate(false);
-    frame->loader()->subframeLoader()->loadMediaPlayerProxyPlugin(mediaElement, kurl, paramNames, paramValues);
-}
-#endif
 
+// FIXME: This should be moved into FrameView (the only caller)
 void RenderEmbeddedObject::updateWidget(bool onlyCreateNonNetscapePlugins)
 {
     if (!m_replacementText.isNull() || !node()) // Check the node in case destroy() has been called.
@@ -201,18 +114,13 @@ void RenderEmbeddedObject::updateWidget(bool onlyCreateNonNetscapePlugins)
     // artifically to ensure that we remain alive for the duration of plug-in initialization.
     RenderWidgetProtector protector(this);
 
-    if (node()->hasTagName(objectTag)) {
-        HTMLObjectElement* objectElement = static_cast<HTMLObjectElement*>(node());
-        updateWidgetForObjectElement(objectElement, onlyCreateNonNetscapePlugins);
-    } else if (node()->hasTagName(embedTag)) {
-        HTMLEmbedElement* embedElement = static_cast<HTMLEmbedElement*>(node());
-        updateWidgetForEmbedElement(embedElement, onlyCreateNonNetscapePlugins);
-    }
+    if (node()->hasTagName(objectTag))
+        static_cast<HTMLObjectElement*>(node())->updateWidget(onlyCreateNonNetscapePlugins);
+    else if (node()->hasTagName(embedTag))
+        static_cast<HTMLEmbedElement*>(node())->updateWidget(onlyCreateNonNetscapePlugins);
 #if ENABLE(PLUGIN_PROXY_FOR_VIDEO)        
-    else if (node()->hasTagName(videoTag) || node()->hasTagName(audioTag)) {
-        HTMLMediaElement* mediaElement = static_cast<HTMLMediaElement*>(node());
-        updateWidgetForMediaElement(mediaElement, onlyCreateNonNetscapePlugins);
-    }
+    else if (node()->hasTagName(videoTag) || node()->hasTagName(audioTag))
+        static_cast<HTMLMediaElement*>(node())->updateWidget(onlyCreateNonNetscapePlugins);
 #endif
     else
         ASSERT_NOT_REACHED();

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list