Bug 1545239 - Unobserve the target if using a different observed box. r=dholbert
authorBoris Chiou <boris.chiou@gmail.com>
Wed, 08 May 2019 20:54:43 +0000
changeset 531971 a0c91ab4d3477cdb041c00dee13354cafafb83fd
parent 531970 b34ea6abce97b002de66d2d0376b84e347f3151a
child 531972 176eced1154bca5f78b54505aa0b8a7bbabeacb6
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1545239
milestone68.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 1545239 - Unobserve the target if using a different observed box. r=dholbert Differential Revision: https://phabricator.services.mozilla.com/D30129
dom/base/ResizeObserver.cpp
testing/web-platform/tests/resize-observer/observe.html
--- a/dom/base/ResizeObserver.cpp
+++ b/dom/base/ResizeObserver.cpp
@@ -140,23 +140,27 @@ already_AddRefed<ResizeObserver> ResizeO
 }
 
 void ResizeObserver::Observe(Element& aTarget,
                              const ResizeObserverOptions& aOptions,
                              ErrorResult& aRv) {
   RefPtr<ResizeObservation> observation;
 
   if (mObservationMap.Get(&aTarget, getter_AddRefs(observation))) {
-    // Already observed this target, so return.
-    // Note: Based on the spec, we should unobserve it first. However, calling
-    // Unobserve() will remove original ResizeObservation and then add a new
-    // one, this may cause an unexpected result because ResizeObservation stores
-    // the last reported size which should be kept to make sure IsActive()
-    // returns the correct result.
-    return;
+    if (observation->BoxOptions() == aOptions.mBox) {
+      // Already observed this target and the observed box is the same, so
+      // return.
+      // Note: Based on the spec, we should unobserve it first. However,
+      // calling Unobserve() when we observe the same box will remove original
+      // ResizeObservation and then add a new one, this may cause an unexpected
+      // result because ResizeObservation stores the mLastReportedSize which
+      // should be kept to make sure IsActive() returns the correct result.
+      return;
+    }
+    Unobserve(aTarget, aRv);
   }
 
   nsIFrame* frame = aTarget.GetPrimaryFrame();
   observation = new ResizeObservation(
       aTarget, aOptions.mBox, frame ? frame->GetWritingMode() : WritingMode());
 
   mObservationMap.Put(&aTarget, observation);
   mObservationList.insertBack(observation);
--- a/testing/web-platform/tests/resize-observer/observe.html
+++ b/testing/web-platform/tests/resize-observer/observe.html
@@ -560,16 +560,88 @@ function test13() {
                       "target content-box block size");
       },
     }
   ]);
 
   return helper.start(() => t.remove());
 }
 
+function test14() {
+  let t = createAndAppendElement("div");
+  t.style.width = "100px";
+  t.style.height = "100px";
+
+  let helper = new ResizeTestHelper(
+    "test14: observe the same target but using a different box should " +
+    "override the previous one",
+  [
+    {
+      setup: observer => {
+        observer.observe(t, { box: "content-box" });
+        observer.observe(t, { box: "border-box" });
+      },
+      notify: entries => {
+        assert_equals(entries.length, 1, "1 pending notification");
+        assert_equals(entries[0].target, t, "target is t");
+        assert_equals(entries[0].contentRect.width, 100, "target width");
+        assert_equals(entries[0].contentRect.height, 100, "target height");
+        assert_equals(entries[0].contentRect.top, 0, "target top padding");
+        assert_equals(entries[0].contentRect.left, 0, "target left padding");
+        assert_equals(entries[0].contentBoxSize.inlineSize, 100,
+                      "target content-box inline size");
+        assert_equals(entries[0].contentBoxSize.blockSize, 100,
+                      "target content-box block size");
+        assert_equals(entries[0].borderBoxSize.inlineSize, 100,
+                      "target border-box inline size");
+        assert_equals(entries[0].borderBoxSize.blockSize, 100,
+                      "target border-box block size");
+      }
+    },
+    {
+      setup: observer => {
+        // Change border-box size.
+        t.style.padding = "4px";
+      },
+      notify: entries => {
+        assert_equals(entries.length, 1, "1 pending notification");
+        assert_equals(entries[0].target, t, "target is t");
+        assert_equals(entries[0].contentRect.width, 100, "target width");
+        assert_equals(entries[0].contentRect.height, 100, "target height");
+        assert_equals(entries[0].contentRect.top, 4, "target top padding");
+        assert_equals(entries[0].contentRect.left, 4, "target left padding");
+        assert_equals(entries[0].contentBoxSize.inlineSize, 100,
+                      "target content-box inline size");
+        assert_equals(entries[0].contentBoxSize.blockSize, 100,
+                      "target content-box block size");
+        assert_equals(entries[0].borderBoxSize.inlineSize, 108,
+                      "target border-box inline size");
+        assert_equals(entries[0].borderBoxSize.blockSize, 108,
+                      "target border-box block size");
+      }
+    },
+    {
+      setup: observer => {
+        // Change only content-box size.
+        t.style.width = "104px";
+        t.style.height = "104px";
+        t.style.padding = "2px";
+      },
+      notify: entries => {
+        assert_unreached("the 'border-box' ResizeObserver shouldn't fire " +
+                         "for restyles that don't affect the border-box size");
+      },
+      timeout: () => {
+        // expected: 104 + 2 * 2 = 108. The border-box size is the same.
+      }
+    }
+  ]);
+  return helper.start(() => t.remove());
+}
+
 let guard;
 test(_ => {
   assert_own_property(window, "ResizeObserver");
   guard = async_test('guard');
 }, "ResizeObserver implemented")
 
 test0()
   .then(() => test1())
@@ -580,11 +652,12 @@ test0()
   .then(() => test6())
   .then(() => test7())
   .then(() => test8())
   .then(() => test9())
   .then(() => test10())
   .then(() => test11())
   .then(() => test12())
   .then(() => test13())
+  .then(() => test14())
   .then(() => guard.done());
 
 </script>