Bug 1441168 - Make promiseDocumentFlushed reject if the DOM is modified in the callback. r=Gijs
☠☠ backed out by d8e076214d14 ☠ ☠
authorMike Conley <mconley@mozilla.com>
Wed, 19 Dec 2018 18:32:18 +0000
changeset 511515 185b7717cc2e
parent 511514 84b78ae21626
child 511516 3ee226bda683
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1441168
milestone66.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 1441168 - Make promiseDocumentFlushed reject if the DOM is modified in the callback. r=Gijs Differential Revision: https://phabricator.services.mozilla.com/D14101
dom/base/nsGlobalWindowInner.cpp
dom/base/test/browser_promiseDocumentFlushed.js
--- a/dom/base/nsGlobalWindowInner.cpp
+++ b/dom/base/nsGlobalWindowInner.cpp
@@ -804,22 +804,26 @@ class PromiseDocumentFlushedResolver fin
  public:
   PromiseDocumentFlushedResolver(Promise* aPromise,
                                  PromiseDocumentFlushedCallback& aCallback)
       : mPromise(aPromise), mCallback(&aCallback) {}
 
   virtual ~PromiseDocumentFlushedResolver() = default;
 
   void Call() {
+    nsMutationGuard guard;
     ErrorResult error;
     JS::Rooted<JS::Value> returnVal(RootingCx());
     mCallback->Call(&returnVal, error);
 
     if (error.Failed()) {
       mPromise->MaybeReject(error);
+    } else if (guard.Mutated(0)) {
+      // Something within the callback mutated the DOM.
+      mPromise->MaybeReject(NS_ERROR_DOM_NO_MODIFICATION_ALLOWED_ERR);
     } else {
       mPromise->MaybeResolve(returnVal);
     }
   }
 
   void Cancel() { mPromise->MaybeReject(NS_ERROR_ABORT); }
 
   RefPtr<Promise> mPromise;
--- a/dom/base/test/browser_promiseDocumentFlushed.js
+++ b/dom/base/test/browser_promiseDocumentFlushed.js
@@ -238,9 +238,28 @@ add_task(async function test_execution_o
 
   Assert.equal(result.length, 8,
     "Should have run all callbacks and Promises.");
 
   for (let i = 0; i < result.length; ++i) {
     Assert.equal(result[i], i,
       "Callbacks and Promises should have run in the expected order.");
   }
+
+  await cleanTheDOM();
 });
+
+/**
+ * Tests that modifying the DOM within a promiseDocumentFlushed callback
+ * will result in the Promise being rejected.
+ */
+add_task(async function test_reject_on_modification() {
+  dirtyStyleAndLayout(1);
+  assertFlushesRequired();
+
+  let promise = window.promiseDocumentFlushed(() => {
+    dirtyStyleAndLayout(2);
+  });
+
+  await Assert.rejects(promise, /NoModificationAllowedError/);
+
+  await cleanTheDOM();
+});