Bug 1332956 part 3. Implement the same behavior for <object>-inside-<object> and <object>-inside-mediaelement as we do for <embed> already. r=qdot

2017-02-08 18:19:01 -05:00
parent 6de9ae9d5d
commit e5460d8ba1
10 changed files with 181 additions and 77 deletions
--- a/dom/base/nsObjectLoadingContent.cpp
+++ b/dom/base/nsObjectLoadingContent.cpp
@@ -88,6 +88,7 @@
 #include "mozilla/Telemetry.h"
 #include "mozilla/dom/HTMLObjectElementBinding.h"
 #include "mozilla/dom/HTMLSharedObjectElement.h"
+#include "mozilla/dom/HTMLObjectElement.h"
 #include "nsChannelClassifier.h"

 #ifdef XP_WIN
@@ -97,13 +98,6 @@
 #endif
 #endif // XP_WIN

-#ifdef XP_MACOSX
-// HandlePluginCrashed() and HandlePluginInstantiated() needed from here to
-// fix bug 1147521.  Should later be replaced by proper interface methods,
-// maybe on nsIObjectLoadingContext.
-#include "mozilla/dom/HTMLObjectElement.h"
-#endif
-
 static NS_DEFINE_CID(kAppShellCID, NS_APPSHELL_CID);

 static const char *kPrefJavaMIME = "plugin.java.mime";
@@ -3031,8 +3025,10 @@ nsObjectLoadingContent::LoadFallback(FallbackType aType, bool aNotify) {
  // child embeds as we find them in the upcoming loop.
  mType = eType_Null;

-  // Do a breadth-first traverse of node tree with the current element as root,
-  // looking for the first embed we can find.
+  bool thisIsObject = thisContent->IsHTMLElement(nsGkAtoms::object);
+
+  // Do a depth-first traverse of node tree with the current element as root,
+  // looking for <embed> or <object> elements that might now need to load.
  nsTArray<nsINodeList*> childNodes;
  if ((thisContent->IsHTMLElement(nsGkAtoms::object) ||
       thisContent->IsHTMLElement(nsGkAtoms::applet)) &&
@@ -3048,10 +3044,11 @@ nsObjectLoadingContent::LoadFallback(FallbackType aType, bool aNotify) {
          nsStyleUtil::IsSignificantChild(child, true, false)) {
        aType = eFallbackAlternate;
      }
-      if (child->IsHTMLElement(nsGkAtoms::embed) &&
-          thisContent->IsHTMLElement(nsGkAtoms::object)) {
-        HTMLSharedObjectElement* object = static_cast<HTMLSharedObjectElement*>(child);
-        if (object) {
+      if (thisIsObject) {
+        if (child->IsHTMLElement(nsGkAtoms::embed)) {
+          HTMLSharedObjectElement* embed = static_cast<HTMLSharedObjectElement*>(child);
+          embed->StartObjectLoad(true, true);
+        } else if (auto object = HTMLObjectElement::FromContent(child)) {
          object->StartObjectLoad(true, true);
        }
      }
@@ -3817,6 +3814,38 @@ nsObjectLoadingContent::MaybeFireErrorEvent()
  }
 }

+bool
+nsObjectLoadingContent::BlockEmbedOrObjectContentLoading()
+{
+  nsCOMPtr<nsIContent> thisContent =
+    do_QueryInterface(static_cast<nsIImageLoadingContent*>(this));
+  if (!thisContent->IsHTMLElement(nsGkAtoms::embed) &&
+      !thisContent->IsHTMLElement(nsGkAtoms::object)) {
+    // Doesn't apply to other elements (i.e. <applet>)
+    return false;
+  }
+
+  // Traverse up the node tree to see if we have any ancestors that may block us
+  // from loading
+  for (nsIContent* parent = thisContent->GetParent();
+       parent;
+       parent = parent->GetParent()) {
+    if (parent->IsAnyOfHTMLElements(nsGkAtoms::video, nsGkAtoms::audio)) {
+      return true;
+    }
+    // If we have an ancestor that is an object with a source, it'll have an
+    // associated displayed type. If that type is not null, don't load content
+    // for the embed.
+    if (HTMLObjectElement* object = HTMLObjectElement::FromContent(parent)) {
+      uint32_t type = object->DisplayedType();
+      if (type != eType_Null) {
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
 // SetupProtoChainRunner implementation
 nsObjectLoadingContent::SetupProtoChainRunner::SetupProtoChainRunner(
    nsObjectLoadingContent* aContent)
--- a/dom/base/nsObjectLoadingContent.h
+++ b/dom/base/nsObjectLoadingContent.h
@@ -334,6 +334,21 @@ class nsObjectLoadingContent : public nsImageLoadingContent
     */
    virtual nsContentPolicyType GetContentPolicyType() const = 0;

+    /**
+     * Decides whether we should load <embed>/<object> node content.
+     *
+     * If this is an <embed> or <object> node there are cases in which we should
+     * not try to load the content:
+     *
+     * - If the node is the child of a media element
+     * - If the node is the child of an <object> node that already has
+     *   content being loaded.
+     *
+     * In these cases, this function will return false, which will cause
+     * us to skip calling LoadObject.
+     */
+    bool BlockEmbedOrObjectContentLoading();
+
  private:

    // Object parameter changes returned by UpdateObjectParameters
--- a/dom/html/HTMLObjectElement.cpp
+++ b/dom/html/HTMLObjectElement.cpp
@@ -79,7 +79,7 @@ HTMLObjectElement::DoneAddingChildren(bool aHaveNotified)
  // If we're already in a document, we need to trigger the load
  // Otherwise, BindToTree takes care of that.
  if (IsInComposedDoc()) {
-    StartObjectLoad(aHaveNotified);
+    StartObjectLoad(aHaveNotified, false);
  }
 }

@@ -310,7 +310,8 @@ HTMLObjectElement::SetAttr(int32_t aNameSpaceID, nsIAtom *aName,
  // a document, just in case that the caller wants to set additional
  // attributes before inserting the node into the document.
  if (aNotify && IsInComposedDoc() && mIsDoneAddingChildren &&
-      aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::data) {
+      aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::data &&
+      !BlockEmbedOrObjectContentLoading()) {
    return LoadObject(aNotify, true);
  }

@@ -327,7 +328,8 @@ HTMLObjectElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,

  // See comment in SetAttr
  if (aNotify && IsInComposedDoc() && mIsDoneAddingChildren &&
-      aNameSpaceID == kNameSpaceID_None && aAttribute == nsGkAtoms::data) {
+      aNameSpaceID == kNameSpaceID_None && aAttribute == nsGkAtoms::data &&
+      !BlockEmbedOrObjectContentLoading()) {
    return LoadObject(aNotify, true);
  }

@@ -535,15 +537,16 @@ HTMLObjectElement::GetAttributeMappingFunction() const
 }

 void
-HTMLObjectElement::StartObjectLoad(bool aNotify)
+HTMLObjectElement::StartObjectLoad(bool aNotify, bool aForce)
 {
  // BindToTree can call us asynchronously, and we may be removed from the tree
  // in the interim
-  if (!IsInComposedDoc() || !OwnerDoc()->IsActive()) {
+  if (!IsInComposedDoc() || !OwnerDoc()->IsActive() ||
+      BlockEmbedOrObjectContentLoading()) {
    return;
  }

-  LoadObject(aNotify);
+  LoadObject(aNotify, aForce);
  SetIsNetworkCreated(false);
 }

--- a/dom/html/HTMLObjectElement.h
+++ b/dom/html/HTMLObjectElement.h
@@ -98,7 +98,7 @@ public:

  nsresult CopyInnerTo(Element* aDest);

-  void StartObjectLoad() { StartObjectLoad(true); }
+  void StartObjectLoad() { StartObjectLoad(true, false); }

  NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(HTMLObjectElement,
                                           nsGenericHTMLFormElement)
@@ -247,12 +247,12 @@ public:
    return GetContentDocument(aSubjectPrincipal);
  }

-private:
  /**
   * Calls LoadObject with the correct arguments to start the plugin load.
   */
-  void StartObjectLoad(bool aNotify);
+  void StartObjectLoad(bool aNotify, bool aForceLoad);

+private:
  /**
   * Returns if the element is currently focusable regardless of it's tabindex
   * value. This is used to know the default tabindex value.
--- a/dom/html/HTMLSharedObjectElement.cpp
+++ b/dom/html/HTMLSharedObjectElement.cpp
@@ -180,7 +180,7 @@ HTMLSharedObjectElement::SetAttr(int32_t aNameSpaceID, nsIAtom *aName,
  // attributes before inserting the node into the document.
  if (aNotify && IsInComposedDoc() && mIsDoneAddingChildren &&
      aNameSpaceID == kNameSpaceID_None && aName == URIAttrName()
-      && !BlockEmbedContentLoading()) {
+      && !BlockEmbedOrObjectContentLoading()) {
    return LoadObject(aNotify, true);
  }

@@ -313,7 +313,7 @@ HTMLSharedObjectElement::StartObjectLoad(bool aNotify, bool aForceLoad)
  // BindToTree can call us asynchronously, and we may be removed from the tree
  // in the interim
  if (!IsInComposedDoc() || !OwnerDoc()->IsActive() ||
-      BlockEmbedContentLoading()) {
+      BlockEmbedOrObjectContentLoading()) {
    return;
  }

@@ -389,31 +389,5 @@ HTMLSharedObjectElement::GetContentPolicyType() const
  }
 }

-bool
-HTMLSharedObjectElement::BlockEmbedContentLoading()
-{
-  // Only check on embed elements
-  if (!IsHTMLElement(nsGkAtoms::embed)) {
-    return false;
-  }
-  // Traverse up the node tree to see if we have any ancestors that may block us
-  // from loading
-  for (nsIContent* parent = GetParent(); parent; parent = parent->GetParent()) {
-    if (parent->IsAnyOfHTMLElements(nsGkAtoms::video, nsGkAtoms::audio)) {
-      return true;
-    }
-    // If we have an ancestor that is an object with a source, it'll have an
-    // associated displayed type. If that type is not null, don't load content
-    // for the embed.
-    if (HTMLObjectElement* object = HTMLObjectElement::FromContent(parent)) {
-      uint32_t type = object->DisplayedType();
-      if (type != eType_Null) {
-        return true;
-      }
-    }
-  }
-  return false;
-}
-
 } // namespace dom
 } // namespace mozilla
--- a/dom/html/HTMLSharedObjectElement.h
+++ b/dom/html/HTMLSharedObjectElement.h
@@ -220,21 +220,6 @@ private:

  static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
                                    GenericSpecifiedValues* aGenericData);
-
-  /**
-   * Decides whether we should load embed node content.
-   *
-   * If this is an embed node there are cases in which we should not try to load
-   * the content:
-   *
-   * - If the embed node is the child of a media element
-   * - If the embed node is the child of an object node that already has
-   *   content being loaded.
-   *
-   * In these cases, this function will return false, which will cause
-   * us to skip calling LoadObject.
-   */
-  bool BlockEmbedContentLoading();
 };

 } // namespace dom
--- a/dom/tests/mochitest/general/resource_timing_cross_origin.html
+++ b/dom/tests/mochitest/general/resource_timing_cross_origin.html
@@ -170,16 +170,16 @@ function finishTests() {
  </a>
  <p id="display"></p>
  <div id="content">
-    <object data="http://mochi.test:8888/tests/dom/tests/mochitest/general/res0.resource"> <!-- same origin, no header -->
-    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res0.resource"> <!-- cross origin, no header -->
-    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res1.resource"> <!-- cross origin, Timing-Allow-Origin: * header -->
-    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res2.resource"> <!-- cross origin redirect to test2.example.com, no header -->
-    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res3.resource"> <!-- cross origin, Timing-Allow-Origin: http://mochi.test:8888 header -->
-    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res4.resource"> <!-- cross origin redirect to mochi.test:8888/.../res1.resource, Timing-Allow-Origin: * -->
-    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res5.resource"> <!-- cross origin, Timing-Allow-Origin: http://mochi.test:8889 -->
-    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res6.resource"> <!-- cross origin, Timing-Allow-Origin: "" (empty string) -->
-    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res7.resource"> <!-- cross origin, Timing-Allow-Origin: http://mochi.test:8888 http://test1.com header -->
-    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res8.resource"> <!-- double cross origin redirect -->
+    <object data="http://mochi.test:8888/tests/dom/tests/mochitest/general/res0.resource"></object> <!-- same origin, no header -->
+    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res0.resource"></object> <!-- cross origin, no header -->
+    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res1.resource"></object> <!-- cross origin, Timing-Allow-Origin: * header -->
+    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res2.resource"></object> <!-- cross origin redirect to test2.example.com, no header -->
+    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res3.resource"></object> <!-- cross origin, Timing-Allow-Origin: http://mochi.test:8888 header -->
+    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res4.resource"></object> <!-- cross origin redirect to mochi.test:8888/.../res1.resource, Timing-Allow-Origin: * -->
+    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res5.resource"></object> <!-- cross origin, Timing-Allow-Origin: http://mochi.test:8889 -->
+    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res6.resource"></object> <!-- cross origin, Timing-Allow-Origin: "" (empty string) -->
+    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res7.resource"></object> <!-- cross origin, Timing-Allow-Origin: http://mochi.test:8888 http://test1.com header -->
+    <object data="http://test1.example.com/tests/dom/tests/mochitest/general/res8.resource"></object> <!-- double cross origin redirect -->
    <script type="text/javascript" src="http://mochi.test:8888/tests/dom/tests/mochitest/general/resource_timing.js"></script> <!-- same origin script -->
  </div>
 </body>
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -92467,6 +92467,18 @@
     {}
    ]
   ],
+   "html/semantics/embedded-content/the-object-element/object-ignored-in-media-element.html": [
+    [
+     "/html/semantics/embedded-content/the-object-element/object-ignored-in-media-element.html",
+     {}
+    ]
+   ],
+   "html/semantics/embedded-content/the-object-element/object-in-object-fallback-2.html": [
+    [
+     "/html/semantics/embedded-content/the-object-element/object-in-object-fallback-2.html",
+     {}
+    ]
+   ],
   "html/semantics/embedded-content/the-object-element/usemap-casing.html": [
    [
     "/html/semantics/embedded-content/the-object-element/usemap-casing.html",
@@ -174549,7 +174561,7 @@
   "support"
  ],
  "html/semantics/embedded-content/the-embed-element/embed-ignored-in-media-element.html": [
-   "94a08a7a5b5ec5c26f1974d5e5d8b4381a60baf5",
+   "cb57cbe52e4f586006461b8eae6bc233b5ed5ad5",
   "testharness"
  ],
  "html/semantics/embedded-content/the-embed-element/embed-in-object-fallback-2.html": [
@@ -174872,6 +174884,14 @@
   "bf051d12a045698b2f9c3870ad4236f65bb85f51",
   "testharness"
  ],
+  "html/semantics/embedded-content/the-object-element/object-ignored-in-media-element.html": [
+   "62a6c079bc00ae6ebeca363fd42d8701c4791222",
+   "testharness"
+  ],
+  "html/semantics/embedded-content/the-object-element/object-in-object-fallback-2.html": [
+   "a5bb885111ac7ea02241957ee7233491c2277516",
+   "testharness"
+  ],
  "html/semantics/embedded-content/the-object-element/test0.html": [
   "04319dea2f1e0b00e8db1703f2072ec22f1a82ad",
   "support"
--- a/testing/web-platform/tests/html/semantics/embedded-content/the-object-element/object-ignored-in-media-element.html
+++ b/testing/web-platform/tests/html/semantics/embedded-content/the-object-element/object-ignored-in-media-element.html
@@ -0,0 +1,22 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>HTML Test: The embed element represents a document</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<meta name="assert" content="Check if the object element is ignored when used inside a media element">
+<script type="application/javascript">
+  var nestingTest = async_test("Test <object> being ignored inside media element");
+  onload = nestingTest.step_func_done(function() {
+    assert_true(true, "We got to a load event without loading things we should not load");
+  });
+</script>
+<body>
+  <video>
+    <object type="text/html" data="../resources/should-not-load.html"
+            test-description="<object> in <video>"></object>
+  </video>
+  <audio>
+    <object type="text/html" data="../resources/should-not-load.html"
+            test-description="<object> in <audio>"></object>
+  </audio>
+</body>
--- a/testing/web-platform/tests/html/semantics/embedded-content/the-object-element/object-in-object-fallback-2.html
+++ b/testing/web-platform/tests/html/semantics/embedded-content/the-object-element/object-in-object-fallback-2.html
@@ -0,0 +1,56 @@
+<!doctype html>
+<html>
+  <head>
+    <meta charset=utf-8>
+    <title></title>
+    <script src=/resources/testharness.js></script>
+    <script src=/resources/testharnessreport.js></script>
+    <script>
+      var loadedCount = 0;
+      var nestingTest = async_test("Test <object> nesting inside <object>");
+      onload = nestingTest.step_func_done(function() {
+        assert_equals(loadedCount, 12, "Should have loaded all should-load elements");
+      });
+    </script>
+    <style>
+      object { display: none }
+    </style>
+  </head>
+  <body>
+    <object data="../resources/should-load.html" style="width: 100px; height: 100px">
+      <object type="text/html" data="../resources/should-not-load.html"
+              test-description="<object> inside <object>"></object>
+    </object>
+    <object style="width: 100px; height: 100px" data="data:application/x-does-not-exist,test">
+      <object type="text/html" data="../resources/should-load.html"></object>
+    </object>
+    <object style="width: 100px; height: 100px" data="data:application/x-does-not-exist,test">
+      <div></div>
+      <object type="text/html" data="../resources/should-load.html"></object>
+    </object>
+    <object style="width: 100px; height: 100px" data="data:application/x-does-not-exist,test">
+      <div>
+        <object type="text/html" data="../resources/should-load.html"></object>
+      </div>
+    </object>
+    <object style="width: 100px; height: 100px" data="data:application/x-does-not-exist,test">
+      <object type="text/html" data="../resources/should-load.html"></object>
+      <object type="text/html" data="../resources/should-load.html"></object>
+      <object data="../resources/should-load.html">
+        <object type="text/html" data="../resources/should-not-load.html"
+                test-description="<object> inside loaded <object> inside non-loaded <object>"></object>
+      </object>
+      <object data="data:application/x-does-not-exist,test">
+        <object type="text/html" data="../resources/should-load.html"></object>
+      </object>
+    </object>
+    <div>
+      <object data="../resources/should-load.html" style="width: 100px; height: 100px"></object>
+      <object type="text/html" data="../resources/should-load.html"></object>
+    </div>
+    <div>
+      <object type="text/html" data="../resources/should-load.html"></object>
+      <object data="../resources/should-load.html" style="width: 100px; height: 100px"></object>
+    </div>
+  </body>
+</html>