Bug 1466260 [wpt PR 11309] - Revert "Cleanup plugin element frames when the layout tree is detached", a=testonly
☠☠ backed out by 45cfaa4b4c0c ☠ ☠
authorEhsan Karamad <ekaramad@chromium.org>
Mon, 25 Jun 2018 21:21:04 +0000
changeset 423999 dcebab7e152c83529cadad4d3e9b469665e69776
parent 423998 a17b8dec2abf5a1beab5e81a3541eb3ea94935da
child 424000 48c118bfefb46f06ec2c6ac8e27bc3e29c588681
push id104729
push userjames@hoppipolla.co.uk
push dateWed, 27 Jun 2018 23:29:57 +0000
treeherdermozilla-inbound@7c351d6f780f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1466260, 11309, 776510, 781880, 996314, 561768, 847216, 846708, 1083500, 563902
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1466260 [wpt PR 11309] - Revert "Cleanup plugin element frames when the layout tree is detached", a=testonly Automatic update from web-platform-testsRevert "Cleanup plugin element frames when the layout tree is detached" This reverts commit 596e6b3281a8cc2c4b646553e4cde169a29a1d1d. Reason for revert: DetachLayoutTree could be called at inopportune times such as style recalc or during layout. This caused several crashes. Original change's description: > Cleanup plugin element frames when the layout tree is detached > > Unlike frames, plugin elements (<embed> and <object>) are updated > when their layout tree is rebuilt. However, due to an oversight, > <embed> or <object> elements displaying subframes would only have > the associated content view (FrameView) torn down, leaving a > dangling content frame. > > This led to some interesting side effects: > - When transitioning from a local frame to a plugin, the content > frame would remain live, with JS still happily running. > - When transitioning from a remote frame to a plugin, > - When navigating a remote frame from one URL to another, the > element would stop updating since the FrameView would be torn > down but a new one would never be created. Note that this was > not (as much of) a problem for local frames, since local frames > re-create the LocalFrameView on every navigation. > With this CL, if a plugin element has an associated content frame, > use that to clean up the state associated with the element when the > layout tree is detached. This can cause the browser context to be > re-created: this matches the behavior of Edge and Firefox. > > Note that there is still one edge case where Blink behaves oddly: > if the navigation fails and should display fallback content, the > content frame still remains attached. This will be addressed in > followups. > > Link to whatwg issue: https://github.com/whatwg/html/issues/3576 > > Bug: 776510, 781880 > Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng > Change-Id: Id18605fbe602ea6c0076c1d579345cdcf28cc984 > Reviewed-on: https://chromium-review.googlesource.com/996314 > Commit-Queue: Ehsan Karamad <ekaramad@chromium.org> > Reviewed-by: Daniel Cheng <dcheng@chromium.org> > Reviewed-by: Alex Moshchuk <alexmos@chromium.org> > Cr-Commit-Position: refs/heads/master@{#561768} TBR=dcheng@chromium.org,creis@chromium.org,alexmos@chromium.org,ekaramad@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 776510, 781880, 847216, 846708 Change-Id: I762631b92463352bea9bf3102d90a13b72776786 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng Reviewed-on: https://chromium-review.googlesource.com/1083500 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Ehsan Karamad <ekaramad@chromium.org> Commit-Queue: Ehsan Karamad <ekaramad@chromium.org> Cr-Commit-Position: refs/heads/master@{#563902} -- wpt-commits: 69a48d90cac792566f14d734d2bae4feda462829 wpt-pr: 11309
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/html/semantics/embedded-content/resources/test_page.html
testing/web-platform/tests/html/semantics/embedded-content/the-embed-element/detach-frame-on-src-change.html
testing/web-platform/tests/html/semantics/embedded-content/the-object-element/detach-frame-on-data-change.html
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -284236,21 +284236,16 @@
      {}
     ]
    ],
    "html/semantics/embedded-content/resources/should-not-load.html": [
     [
      {}
     ]
    ],
-   "html/semantics/embedded-content/resources/test_page.html": [
-    [
-     {}
-    ]
-   ],
    "html/semantics/embedded-content/svg/.gitkeep": [
     [
      {}
     ]
    ],
    "html/semantics/embedded-content/the-area-element/.gitkeep": [
     [
      {}
@@ -343967,22 +343962,16 @@
     ]
    ],
    "html/semantics/embedded-content/the-canvas-element/type.replace.html": [
     [
      "/html/semantics/embedded-content/the-canvas-element/type.replace.html",
      {}
     ]
    ],
-   "html/semantics/embedded-content/the-embed-element/detach-frame-on-src-change.html": [
-    [
-     "/html/semantics/embedded-content/the-embed-element/detach-frame-on-src-change.html",
-     {}
-    ]
-   ],
    "html/semantics/embedded-content/the-embed-element/document-getters-return-null-for-cross-origin.html": [
     [
      "/html/semantics/embedded-content/the-embed-element/document-getters-return-null-for-cross-origin.html",
      {}
     ]
    ],
    "html/semantics/embedded-content/the-embed-element/embed-dimension.html": [
     [
@@ -344547,22 +344536,16 @@
     ]
    ],
    "html/semantics/embedded-content/the-img-element/usemap-casing.html": [
     [
      "/html/semantics/embedded-content/the-img-element/usemap-casing.html",
      {}
     ]
    ],
-   "html/semantics/embedded-content/the-object-element/detach-frame-on-data-change.html": [
-    [
-     "/html/semantics/embedded-content/the-object-element/detach-frame-on-data-change.html",
-     {}
-    ]
-   ],
    "html/semantics/embedded-content/the-object-element/document-getters-return-null-for-cross-origin.html": [
     [
      "/html/semantics/embedded-content/the-object-element/document-getters-return-null-for-cross-origin.html",
      {}
     ]
    ],
    "html/semantics/embedded-content/the-object-element/historical.html": [
     [
@@ -580342,20 +580325,16 @@
   "html/semantics/embedded-content/resources/should-load.html": [
    "7cc95b9b2eb8c3f2d89278d22db271b9ae42e107",
    "support"
   ],
   "html/semantics/embedded-content/resources/should-not-load.html": [
    "a1657fc9e655ad0a30ced47a1412b6c34ba964b9",
    "support"
   ],
-  "html/semantics/embedded-content/resources/test_page.html": [
-   "7865ad89746586ea71fa92621192417de57223ca",
-   "support"
-  ],
   "html/semantics/embedded-content/svg/.gitkeep": [
    "da39a3ee5e6b4b0d3255bfef95601890afd80709",
    "support"
   ],
   "html/semantics/embedded-content/the-area-element/.gitkeep": [
    "da39a3ee5e6b4b0d3255bfef95601890afd80709",
    "support"
   ],
@@ -581070,20 +581049,16 @@
   "html/semantics/embedded-content/the-canvas-element/type.replace.html": [
    "c8784361857ba83afbe1be4bca4ed5e654307c2f",
    "testharness"
   ],
   "html/semantics/embedded-content/the-embed-element/.gitkeep": [
    "da39a3ee5e6b4b0d3255bfef95601890afd80709",
    "support"
   ],
-  "html/semantics/embedded-content/the-embed-element/detach-frame-on-src-change.html": [
-   "55e68ad2312d4079ce4be558ef361f6f56977d58",
-   "testharness"
-  ],
   "html/semantics/embedded-content/the-embed-element/document-getters-return-null-for-cross-origin.html": [
    "3d75c8c998b3f98f2023311826dcd1d4c21d3361",
    "testharness"
   ],
   "html/semantics/embedded-content/the-embed-element/embed-dimension.html": [
    "6f8443aada05e8ab3cbb81d81d7053a81fe0dcfe",
    "testharness"
   ],
@@ -581814,20 +581789,16 @@
   "html/semantics/embedded-content/the-map-element/.gitkeep": [
    "da39a3ee5e6b4b0d3255bfef95601890afd80709",
    "support"
   ],
   "html/semantics/embedded-content/the-object-element/.gitkeep": [
    "da39a3ee5e6b4b0d3255bfef95601890afd80709",
    "support"
   ],
-  "html/semantics/embedded-content/the-object-element/detach-frame-on-data-change.html": [
-   "c534d05044768f72015e9cea7dd45b582ecf0368",
-   "testharness"
-  ],
   "html/semantics/embedded-content/the-object-element/document-getters-return-null-for-cross-origin.html": [
    "f9f22673579147c7591d5f71467d6655140702b9",
    "testharness"
   ],
   "html/semantics/embedded-content/the-object-element/historical.html": [
    "3efc9442872199305ef71c7c29331ae465fb8395",
    "testharness"
   ],
deleted file mode 100644
--- a/testing/web-platform/tests/html/semantics/embedded-content/resources/test_page.html
+++ /dev/null
@@ -1,3 +0,0 @@
-<!doctype html>
-<title> An empty test page </title>
-<p> This is test page </p>
deleted file mode 100644
--- a/testing/web-platform/tests/html/semantics/embedded-content/the-embed-element/detach-frame-on-src-change.html
+++ /dev/null
@@ -1,82 +0,0 @@
-<!doctype html>
-<html>
-<head>
-  <title>
-    &lt;embed&gt;'s browsing context is discarded on 'src' attribute change.
-  </title>
-  <link rel="author" title="Ehsan Karamad" href="ekaramad@chromium.org">
-  <script src="/resources/testharness.js"></script>
-  <script src="/resources/testharnessreport.js"></script>
-</head>
-<body>
-  <script>
-    let url1 = "../resources/test_page.html";
-    let url2 = "../resources/should-load.html";
-    function onLoadPromise(el) {
-      return new Promise((resolve) => {
-        function onLoad() {
-          resolve();
-          el.removeEventListener("load", onLoad);
-        }
-        el.addEventListener("load", onLoad);
-      });
-    }
-
-    promise_test(async() => {
-      let old_windows = [];
-
-      let embed = document.createElement("embed");
-      embed.type = "text/html";
-      embed.src = url1;
-      let onEmbedLoad = onLoadPromise(embed);
-      document.body.appendChild(embed);
-      await onEmbedLoad;
-      old_windows.push(window[0]);
-      assert_equals(
-        window[0].frameElement,
-        embed,
-        "<embed> is attached and loaded with html content.");
-
-      let iframe = document.createElement("iframe");
-      iframe.src = url1;
-      let onIframeLoad = onLoadPromise(iframe);
-      document.body.appendChild(iframe);
-      await onIframeLoad;
-      old_windows.push(window[1]);
-      assert_equals(
-        window[1].frameElement,
-        iframe,
-        "<iframe> is attached and loaded with html content after <embed>.");
-      assert_equals(
-        window[0],
-        old_windows[0],
-        "The first window is that of <embed>'s frame.");
-
-      // Now navigate the embed element again.
-      onEmbedLoad = onLoadPromise(embed);
-      embed.src = url2;
-      await onEmbedLoad;
-      assert_equals(
-        window[0].frameElement,
-        iframe,
-        "<embed>'s previous frame must have been destroyed.");
-
-      assert_equals(
-        window[1].frameElement,
-        embed,
-        "<embed>'s new window should be appended after navigation.");
-
-      assert_not_equals(
-        old_windows[0],
-        window[1],
-        "The old window and new window are different for <embed>.");
-
-      assert_equals(
-        old_windows[1],
-        window[0],
-        "The old and new window are the same for <iframe>.");
-    }, "Verify that changing 'src' attribute of an <embed> element discards" +
-       " the old browsing context and creates a new browsing context.");
-  </script>
-</body>
-</html>
deleted file mode 100644
--- a/testing/web-platform/tests/html/semantics/embedded-content/the-object-element/detach-frame-on-data-change.html
+++ /dev/null
@@ -1,83 +0,0 @@
-<!doctype html>
-<html>
-<head>
-  <title>
-    &lt;object&gt;'s browsing context is discarded on 'data' attribute change.
-  </title>
-  <link rel="author" title="Ehsan Karamad" href="ekaramad@chromium.org">
-  <script src="/resources/testharness.js"></script>
-  <script src="/resources/testharnessreport.js"></script>
-</head>
-<body>
-  <script>
-    let url1 = "../resources/test_page.html";
-    let url2 = "../resources/should-load.html";
-    function onLoadPromise(el) {
-      return new Promise((resolve) => {
-        function onLoad() {
-          resolve();
-          el.removeEventListener("load", onLoad);
-        }
-        el.addEventListener("load", onLoad);
-      });
-    }
-
-    promise_test(async() => {
-      let old_windows = [];
-
-      let object = document.createElement("object");
-      object.type = "text/html";
-      object.data = url1;
-      let onObjectLoad = onLoadPromise(object);
-      document.body.appendChild(object);
-      await onObjectLoad;
-      old_windows.push(window[0]);
-      assert_equals(
-        window[0].frameElement,
-        object,
-        "<object> is attached and loaded with html content.");
-
-      let iframe = document.createElement("iframe");
-      iframe.src = url1;
-      let onIframeLoad = onLoadPromise(iframe);
-      document.body.appendChild(iframe);
-      await onIframeLoad;
-      old_windows.push(window[1]);
-      assert_equals(
-        window[1].frameElement,
-        iframe,
-        "<iframe> is attached and loaded with html content after <object>.");
-      assert_equals(
-        window[0],
-        old_windows[0],
-        "The first window is that of <object>'s frame.");
-
-      // Now navigate the object element again.
-      onObjectLoad = onLoadPromise(object);
-      object.data = url2;
-      await onObjectLoad;
-      assert_equals(
-        window[0].frameElement,
-        iframe,
-        "<object>'s previous frame must have been destroyed.");
-
-      assert_equals(
-        window[1].frameElement,
-        object,
-        "<object>'s new window should be appended after navigation.");
-
-      assert_not_equals(
-        old_windows[0],
-        window[1],
-        "The old window and new window are different for <object>.");
-
-      assert_equals(
-        old_windows[1],
-        window[0],
-        "The old and new window are the same for <iframe>.");
-
-    }, "Verify that changing 'data' attribute of an <object> element discards" +
-       " the old browsing context and creates a new browsing context.");
-  </script>
-</body>
-</html>