RenderEmbeddedObject shouldn't know about fallback content.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Nov 2013 09:29:50 +0000 (09:29 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Nov 2013 09:29:50 +0000 (09:29 +0000)
<https://webkit.org/b/123781>

Stop caching the presence of fallback (DOM) content in a flag on
RenderEmbeddedObject and have SubframeLoader fetch it directly from
HTMLObjectElement instead.

Also made SubframeLoader::requestObject() take the owner element
by reference since we don't support owner-less embedded objects.

Reviewed by Antti Koivisto.

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

Source/WebCore/ChangeLog
Source/WebCore/html/HTMLEmbedElement.cpp
Source/WebCore/html/HTMLObjectElement.cpp
Source/WebCore/html/HTMLObjectElement.h
Source/WebCore/loader/SubframeLoader.cpp
Source/WebCore/loader/SubframeLoader.h
Source/WebCore/rendering/RenderEmbeddedObject.cpp
Source/WebCore/rendering/RenderEmbeddedObject.h

index d460217..682e9b4 100644 (file)
@@ -1,3 +1,17 @@
+2013-11-05  Andreas Kling  <akling@apple.com>
+
+        RenderEmbeddedObject shouldn't know about fallback content.
+        <https://webkit.org/b/123781>
+
+        Stop caching the presence of fallback (DOM) content in a flag on
+        RenderEmbeddedObject and have SubframeLoader fetch it directly from
+        HTMLObjectElement instead.
+
+        Also made SubframeLoader::requestObject() take the owner element
+        by reference since we don't support owner-less embedded objects.
+
+        Reviewed by Antti Koivisto.
+
 2013-11-05  Xabier Rodriguez Calvar  <calvaris@igalia.com>
 
         [GStreamer] Remove NATIVE_FULLSCREEN_VIDEO support
index ef63ae0..f369ef4 100644 (file)
@@ -172,7 +172,7 @@ void HTMLEmbedElement::updateWidget(PluginCreationOption pluginCreationOption)
 
     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, getNameAttribute(), m_serviceType, paramNames, paramValues);
+    loader.requestObject(*this, m_url, getNameAttribute(), m_serviceType, paramNames, paramValues);
 }
 
 bool HTMLEmbedElement::rendererIsNeeded(const RenderStyle& style)
index 9a133ce..d27cb27 100644 (file)
@@ -294,9 +294,6 @@ void HTMLObjectElement::updateWidget(PluginCreationOption pluginCreationOption)
     if (!allowedToLoadFrameURL(url))
         return;
 
-    bool fallbackContent = hasFallbackContent();
-    renderEmbeddedObject()->setHasFallbackContent(fallbackContent);
-
     // FIXME: It's sadness that we have this special case here.
     //        See http://trac.webkit.org/changeset/25128 and
     //        plugins/netscape-plugin-setwindow-size.html
@@ -312,8 +309,8 @@ void HTMLObjectElement::updateWidget(PluginCreationOption pluginCreationOption)
         return;
 
     SubframeLoader& loader = document().frame()->loader().subframeLoader();
-    bool success = beforeLoadAllowedLoad && hasValidClassId() && loader.requestObject(this, url, getNameAttribute(), serviceType, paramNames, paramValues);
-    if (!success && fallbackContent)
+    bool success = beforeLoadAllowedLoad && hasValidClassId() && loader.requestObject(*this, url, getNameAttribute(), serviceType, paramNames, paramValues);
+    if (!success && hasFallbackContent())
         renderFallbackContent();
 }
 
index e4599ad..e77b304 100644 (file)
@@ -63,6 +63,8 @@ public:
 
     virtual bool canContainRangeEndPoint() const { return useFallbackContent(); }
 
+    bool hasFallbackContent() const;
+
 private:
     HTMLObjectElement(const QualifiedName&, Document&, HTMLFormElement*, bool createdByParser);
 
@@ -87,8 +89,6 @@ private:
     virtual void updateWidget(PluginCreationOption);
     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);
index e05af61..2d202ed 100644 (file)
@@ -43,7 +43,7 @@
 #include "HTMLAppletElement.h"
 #include "HTMLFrameElementBase.h"
 #include "HTMLNames.h"
-#include "HTMLPlugInImageElement.h"
+#include "HTMLObjectElement.h"
 #include "MIMETypeRegistry.h"
 #include "Page.h"
 #include "PluginData.h"
@@ -208,24 +208,20 @@ static void logPluginRequest(Page* page, const String& mimeType, const String& u
     page->sawPlugin(description);
 }
 
-bool SubframeLoader::requestObject(HTMLPlugInImageElement* ownerElement, const String& url, const AtomicString& frameName, const String& mimeType, const Vector<String>& paramNames, const Vector<String>& paramValues)
+bool SubframeLoader::requestObject(HTMLPlugInImageElement& ownerElement, const String& url, const AtomicString& frameName, const String& mimeType, const Vector<String>& paramNames, const Vector<String>& paramValues)
 {
     if (url.isEmpty() && mimeType.isEmpty())
         return false;
 
-    // FIXME: None of this code should use renderers!
-    RenderEmbeddedObject* renderer = ownerElement->renderEmbeddedObject();
-    ASSERT(renderer);
-    if (!renderer)
-        return false;
-
     URL completedURL;
     if (!url.isEmpty())
         completedURL = completeURL(url);
 
+    bool hasFallbackContent = isHTMLObjectElement(ownerElement) && toHTMLObjectElement(ownerElement).hasFallbackContent();
+
     bool useFallback;
-    if (shouldUsePlugin(completedURL, mimeType, ownerElement->shouldPreferPlugInsForImages(), renderer->hasFallbackContent(), useFallback)) {
-        bool success = requestPlugin(ownerElement, completedURL, mimeType, paramNames, paramValues, useFallback);
+    if (shouldUsePlugin(completedURL, mimeType, ownerElement.shouldPreferPlugInsForImages(), hasFallbackContent, useFallback)) {
+        bool success = requestPlugin(&ownerElement, completedURL, mimeType, paramNames, paramValues, useFallback);
         logPluginRequest(document()->page(), mimeType, completedURL, success);
         return success;
     }
@@ -233,7 +229,7 @@ bool SubframeLoader::requestObject(HTMLPlugInImageElement* ownerElement, const S
     // If the plug-in element already contains a subframe, loadOrRedirectSubframe will re-use it. Otherwise,
     // it will create a new frame and set it as the RenderWidget's Widget, causing what was previously 
     // in the widget to be torn down.
-    return loadOrRedirectSubframe(ownerElement, completedURL, frameName, true, true);
+    return loadOrRedirectSubframe(&ownerElement, completedURL, frameName, true, true);
 }
 
 #if ENABLE(PLUGIN_PROXY_FOR_VIDEO)
index 31c4623..9800900 100644 (file)
@@ -62,7 +62,7 @@ public:
     void clear();
 
     bool requestFrame(HTMLFrameOwnerElement*, const String& url, const AtomicString& frameName, bool lockHistory = true, bool lockBackForwardList = true);    
-    bool requestObject(HTMLPlugInImageElement*, const String& url, const AtomicString& frameName,
+    bool requestObject(HTMLPlugInImageElement&, const String& url, const AtomicString& frameName,
         const String& serviceType, const Vector<String>& paramNames, const Vector<String>& paramValues);
 
 #if ENABLE(PLUGIN_PROXY_FOR_VIDEO)
index c901713..72a9231 100644 (file)
@@ -102,7 +102,6 @@ static const Color& unavailablePluginBorderColor()
 
 RenderEmbeddedObject::RenderEmbeddedObject(HTMLFrameOwnerElement& element, PassRef<RenderStyle> style)
     : RenderWidget(element, std::move(style))
-    , m_hasFallbackContent(false)
     , m_isPluginUnavailable(false)
     , m_isUnavailablePluginIndicatorHidden(false)
     , m_unavailablePluginIndicatorIsPressed(false)
index 11927b6..2fdbce3 100644 (file)
@@ -54,10 +54,6 @@ public:
 
     void setUnavailablePluginIndicatorIsHidden(bool);
 
-    // FIXME: This belongs on HTMLObjectElement.
-    bool hasFallbackContent() const { return m_hasFallbackContent; }
-    void setHasFallbackContent(bool hasFallbackContent) { m_hasFallbackContent = hasFallbackContent; }
-
     void handleUnavailablePluginIndicatorEvent(Event*);
 
     bool isReplacementObscured() const;
@@ -100,8 +96,6 @@ private:
     virtual bool canHaveChildren() const OVERRIDE FINAL;
     virtual bool canHaveWidget() const { return true; }
 
-    bool m_hasFallbackContent; // FIXME: This belongs on HTMLObjectElement.
-
     bool m_isPluginUnavailable;
     bool m_isUnavailablePluginIndicatorHidden;
     PluginUnavailabilityReason m_pluginUnavailabilityReason;