Backed out 3 changesets (bug 1512838) for memory leaks. CLOSED TREE
authorCsoregi Natalia <ncsoregi@mozilla.com>
Sat, 16 Mar 2019 12:20:45 +0200
changeset 464689 8f2a6dbf221ca51fcb80f1a94eea41d377a0c91c
parent 464688 38849d9496e669b52277a069804786effb44fb2c
child 464690 1b9bfa6dab28e2824ea695dedc76f64e0636d0f3
push id112465
push useraciure@mozilla.com
push dateSun, 17 Mar 2019 09:50:10 +0000
treeherdermozilla-inbound@e0861be8d6c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1512838
milestone67.0a1
backs out589f41b2e2536f28eba24afdab49916635a9bf4e
6bd80d61cee83fb8a24e98317c1b1594b3be5c25
754f833aaa41a015efbe682e17313e38d765ba94
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
Backed out 3 changesets (bug 1512838) for memory leaks. CLOSED TREE Backed out changeset 589f41b2e253 (bug 1512838) Backed out changeset 6bd80d61cee8 (bug 1512838) Backed out changeset 754f833aaa41 (bug 1512838)
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/apz/src/APZCTreeManager.h
gfx/layers/apz/src/AsyncPanZoomController.cpp
gfx/layers/apz/test/mochitest/apz_test_utils.js
gfx/layers/apz/test/mochitest/helper_basic_onetouchpinch.html
gfx/layers/apz/test/mochitest/helper_onetouchpinch_nested.html
gfx/layers/apz/test/mochitest/test_group_zoom.html
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -1643,17 +1643,17 @@ APZCTreeManager::GetTouchInputBlockAPZC(
     aOutTouchBehaviors->AppendElement(ConvertToTouchBehavior(hitResult));
   }
   for (size_t i = 1; i < aEvent.mTouches.Length(); i++) {
     RefPtr<AsyncPanZoomController> apzc2 =
         GetTargetAPZC(aEvent.mTouches[i].mScreenPoint, &hitResult, nullptr);
     if (aOutTouchBehaviors) {
       aOutTouchBehaviors->AppendElement(ConvertToTouchBehavior(hitResult));
     }
-    apzc = GetZoomableTarget(apzc, apzc2);
+    apzc = GetMultitouchTarget(apzc, apzc2);
     APZCTM_LOG("Using APZC %p as the root APZC for multi-touch\n", apzc.get());
     // A multi-touch gesture will not be a scrollbar drag, even if the
     // first touch point happened to hit a scrollbar.
     aOutHitScrollbarNode->Clear();
   }
 
   if (aOutHitResult) {
     // XXX we should probably be combining the hit results from the different
@@ -2102,17 +2102,17 @@ void APZCTreeManager::SetTargetAPZC(
   APZThreadUtils::AssertOnControllerThread();
 
   RefPtr<AsyncPanZoomController> target = nullptr;
   if (aTargets.Length() > 0) {
     target = GetTargetAPZC(aTargets[0]);
   }
   for (size_t i = 1; i < aTargets.Length(); i++) {
     RefPtr<AsyncPanZoomController> apzc = GetTargetAPZC(aTargets[i]);
-    target = GetZoomableTarget(target, apzc);
+    target = GetMultitouchTarget(target, apzc);
   }
   mInputQueue->SetConfirmedTargetApzc(aInputBlockId, target);
 }
 
 void APZCTreeManager::UpdateZoomConstraints(
     const ScrollableLayerGuid& aGuid,
     const Maybe<ZoomConstraints>& aConstraints) {
   if (!GetUpdater()->IsUpdaterThread()) {
@@ -2785,21 +2785,16 @@ AsyncPanZoomController* APZCTreeManager:
       mRootNode.get(), [aLayersId](HitTestingTreeNode* aNode) {
         AsyncPanZoomController* apzc = aNode->GetApzc();
         return apzc && apzc->GetLayersId() == aLayersId &&
                apzc->IsRootForLayersId();
       });
   return resultNode ? resultNode->GetApzc() : nullptr;
 }
 
-already_AddRefed<AsyncPanZoomController> APZCTreeManager::FindZoomableApzc(
-    AsyncPanZoomController* aStart) const {
-  return GetZoomableTarget(aStart, aStart);
-}
-
 AsyncPanZoomController* APZCTreeManager::FindRootContentApzcForLayersId(
     LayersId aLayersId) const {
   mTreeLock.AssertCurrentThreadIn();
 
   HitTestingTreeNode* resultNode = BreadthFirstSearch<ReverseIterator>(
       mRootNode.get(), [aLayersId](HitTestingTreeNode* aNode) {
         AsyncPanZoomController* apzc = aNode->GetApzc();
         return apzc && apzc->GetLayersId() == aLayersId &&
@@ -3017,17 +3012,17 @@ ParentLayerToScreenMatrix4x4 APZCTreeMan
 
   return ViewAs<ParentLayerToScreenMatrix4x4>(result);
 }
 
 ScreenPoint APZCTreeManager::GetCurrentMousePosition() const {
   return mCurrentMousePosition;
 }
 
-already_AddRefed<AsyncPanZoomController> APZCTreeManager::GetZoomableTarget(
+already_AddRefed<AsyncPanZoomController> APZCTreeManager::GetMultitouchTarget(
     AsyncPanZoomController* aApzc1, AsyncPanZoomController* aApzc2) const {
   RecursiveMutexAutoLock lock(mTreeLock);
   RefPtr<AsyncPanZoomController> apzc;
   // For now, we only ever want to do pinching on the root-content APZC for
   // a given layers id.
   if (aApzc1 && aApzc2 && aApzc1->GetLayersId() == aApzc2->GetLayersId()) {
     // If the two APZCs have the same layers id, find the root-content APZC
     // for that layers id. Don't call CommonAncestor() because there may not
--- a/gfx/layers/apz/src/APZCTreeManager.h
+++ b/gfx/layers/apz/src/APZCTreeManager.h
@@ -605,23 +605,16 @@ class APZCTreeManager : public IAPZCTree
    * relative to the screen even though no scrolling is occurring.
    * Note that this function expects "spatial coordinates" (i.e. toolbar
    * moves up --> negative delta).
    */
   void ProcessDynamicToolbarMovement(uint32_t aStartTimestampMs,
                                      uint32_t aEndTimestampMs,
                                      ScreenCoord aDeltaY);
 
-  /**
-   * Find the zoomable APZC in the same layer subtree (i.e. with the same
-   * layers id) as the given APZC.
-   */
-  already_AddRefed<AsyncPanZoomController> FindZoomableApzc(
-      AsyncPanZoomController* aStart) const;
-
  private:
   typedef bool (*GuidComparator)(const ScrollableLayerGuid&,
                                  const ScrollableLayerGuid&);
 
   /* Helpers */
   template <class ScrollNode>
   void UpdateHitTestingTreeImpl(LayersId aRootLayerTreeId,
                                 const ScrollNode& aRoot, bool aIsFirstPaint,
@@ -645,17 +638,17 @@ class APZCTreeManager : public IAPZCTree
   already_AddRefed<AsyncPanZoomController> GetAPZCAtPointWR(
       const ScreenPoint& aHitTestPoint,
       gfx::CompositorHitTestInfo* aOutHitResult, LayersId* aOutLayersId,
       HitTestingTreeNode** aOutScrollbarNode);
   AsyncPanZoomController* FindRootApzcForLayersId(LayersId aLayersId) const;
   AsyncPanZoomController* FindRootContentApzcForLayersId(
       LayersId aLayersId) const;
   AsyncPanZoomController* FindRootContentOrRootApzc() const;
-  already_AddRefed<AsyncPanZoomController> GetZoomableTarget(
+  already_AddRefed<AsyncPanZoomController> GetMultitouchTarget(
       AsyncPanZoomController* aApzc1, AsyncPanZoomController* aApzc2) const;
   already_AddRefed<AsyncPanZoomController> CommonAncestor(
       AsyncPanZoomController* aApzc1, AsyncPanZoomController* aApzc2) const;
   /**
    * Perform hit testing for a touch-start event.
    *
    * @param aEvent The touch-start event.
    *
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -1164,19 +1164,16 @@ nsEventStatus AsyncPanZoomController::Ha
       if (!scrollInput.TransformToLocal(aTransformToApzc)) {
         return rv;
       }
 
       rv = OnScrollWheel(scrollInput);
       break;
     }
     case PINCHGESTURE_INPUT: {
-      // The APZCTreeManager should take care of ensuring that only root-content
-      // APZCs get pinch inputs.
-      MOZ_ASSERT(IsRootContent());
       PinchGestureInput pinchInput = aEvent.AsPinchGestureInput();
       if (!pinchInput.TransformToLocal(aTransformToApzc)) {
         return rv;
       }
 
       rv = HandleGestureEvent(pinchInput);
       break;
     }
@@ -1202,28 +1199,16 @@ nsEventStatus AsyncPanZoomController::Ha
 nsEventStatus AsyncPanZoomController::HandleGestureEvent(
     const InputData& aEvent) {
   APZThreadUtils::AssertOnControllerThread();
 
   nsEventStatus rv = nsEventStatus_eIgnore;
 
   switch (aEvent.mInputType) {
     case PINCHGESTURE_INPUT: {
-      // This may be invoked via a one-touch-pinch gesture from
-      // GestureEventListener. In that case we want redirect it to the enclosing
-      // root-content APZC.
-      if (!IsRootContent()) {
-        if (APZCTreeManager* treeManagerLocal = GetApzcTreeManager()) {
-          if (RefPtr<AsyncPanZoomController> root =
-                  treeManagerLocal->FindZoomableApzc(this)) {
-            rv = root->HandleGestureEvent(aEvent);
-          }
-        }
-        break;
-      }
       PinchGestureInput pinchGestureInput = aEvent.AsPinchGestureInput();
       pinchGestureInput.TransformToLocal(GetTransformToThis());
       switch (pinchGestureInput.mType) {
         case PinchGestureInput::PINCHGESTURE_START:
           rv = OnScaleBegin(pinchGestureInput);
           break;
         case PinchGestureInput::PINCHGESTURE_SCALE:
           rv = OnScale(pinchGestureInput);
--- a/gfx/layers/apz/test/mochitest/apz_test_utils.js
+++ b/gfx/layers/apz/test/mochitest/apz_test_utils.js
@@ -52,19 +52,16 @@ function convertBuckets(buckets) {
 
 function convertTestData(testData) {
   var result = {};
   result.paints = convertBuckets(testData.paints);
   result.repaintRequests = convertBuckets(testData.repaintRequests);
   return result;
 }
 
-// Returns the last bucket that has at least one scrollframe. This
-// is useful for skipping over buckets that are from empty transactions,
-// because those don't contain any useful data.
 function getLastNonemptyBucket(buckets) {
   for (var i = buckets.length - 1; i >= 0; --i) {
     if (buckets[i].scrollFrames.length > 0) {
       return buckets[i];
     }
   }
   return null;
 }
@@ -124,19 +121,18 @@ function findRcdNode(apzcTree) {
   return null;
 }
 
 // Return whether an element whose id includes |elementId| has been layerized.
 // Assumes |elementId| will be present in the content description for the
 // element, and not in the content descriptions of other elements.
 function isLayerized(elementId) {
   var contentTestData = SpecialPowers.getDOMWindowUtils(window).getContentAPZTestData();
-  var nonEmptyBucket = getLastNonemptyBucket(contentTestData.paints);
-  ok(nonEmptyBucket != null, "expected at least one nonempty paint");
-  var seqno = nonEmptyBucket.sequenceNumber;
+  ok(contentTestData.paints.length > 0, "expected at least one paint");
+  var seqno = contentTestData.paints[contentTestData.paints.length - 1].sequenceNumber;
   contentTestData = convertTestData(contentTestData);
   var paint = contentTestData.paints[seqno];
   for (var scrollId in paint) {
     if ("contentDescription" in paint[scrollId]) {
       if (paint[scrollId].contentDescription.includes(elementId)) {
         return true;
       }
     }
deleted file mode 100644
--- a/gfx/layers/apz/test/mochitest/helper_basic_onetouchpinch.html
+++ /dev/null
@@ -1,94 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-  <meta charset="utf-8">
-  <meta name="viewport" content="width=device-width">
-  <title>Sanity check for one-touch pinch zooming</title>
-  <script type="application/javascript" src="apz_test_native_event_utils.js"></script>
-  <script type="application/javascript" src="apz_test_utils.js"></script>
-  <script type="application/javascript" src="/tests/SimpleTest/paint_listener.js"></script>
-  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
-  <script type="application/javascript">
-
-function* test(testDriver) {
-  let visResEvt = new EventCounter(window.visualViewport, "resize");
-  let visScrEvt = new EventCounter(window.visualViewport, "scroll");
-  // Our internal visual viewport events aren't restricted to the visual view-
-  // port itself, so we can listen on the window itself, however the event
-  // listener needs to be in the system group.
-  let visResEvtInternal = new EventCounter(window, "mozvisualresize",
-                                           { mozSystemGroup: true });
-  let visScrEvtInternal = new EventCounter(window, "mozvisualscroll",
-                                           { mozSystemGroup: true });
-  let visResEvtContent = new EventCounter(window, "mozvisualresize");
-  let visScrEvtContent = new EventCounter(window, "mozvisualscroll");
-
-  var initial_resolution = getResolution();
-  ok(initial_resolution > 0,
-      "The initial_resolution is " + initial_resolution + ", which is some sane value");
-
-  // This listener will trigger the test to continue once APZ is done with
-  // processing the scroll.
-  SpecialPowers.Services.obs.addObserver(testDriver, "APZ:TransformEnd");
-
-  var zoom_in = [
-      [ { x: 150, y: 300 } ],
-      [ null ],
-      [ { x: 150, y: 300 } ],
-      [ { x: 150, y: 305 } ],
-      [ { x: 150, y: 310 } ],
-      [ { x: 150, y: 315 } ],
-      [ { x: 150, y: 320 } ],
-      [ { x: 150, y: 325 } ],
-  ];
-
-  var touchIds = [0];
-  yield* synthesizeNativeTouchSequences(document.body, zoom_in, null, touchIds);
-
-  // Wait for the APZ:TransformEnd to be fired after touch events are processed.
-  yield true;
-
-  // We get here once the APZ:TransformEnd has fired, so we don't need that
-  // observer any more.
-  SpecialPowers.Services.obs.removeObserver(testDriver, "APZ:TransformEnd");
-
-  // Flush state and get the resolution we're at now
-  yield waitForApzFlushedRepaints(testDriver);
-  let final_resolution = getResolution();
-  ok(final_resolution > initial_resolution, "The final resolution (" + final_resolution + ") is greater after zooming in");
-
-  // Check we've got the expected events.
-  // Zooming the page should fire visual viewport resize events:
-  visResEvt.unregister();
-  ok(visResEvt.count > 0, "Got some visual viewport resize events");
-  visResEvtInternal.unregister();
-  ok(visResEvtInternal.count > 0, "Got some mozvisualresize events");
-
-  // We're zooming somewhere in the middle of the page, so the visual
-  // viewport's coordinates change, too.
-  // This is true both relative to the page (mozvisualscroll), as well as
-  // relative to the layout viewport (visual viewport "scroll" event).
-  visScrEvt.unregister();
-  ok(visScrEvt.count > 0, "Got some visual viewport scroll events");
-  visScrEvtInternal.unregister();
-  ok(visScrEvtInternal.count > 0, "Got some mozvisualscroll events");
-
-  // Our internal events shouldn't leak to normal content.
-  visResEvtContent.unregister();
-  is(visResEvtContent.count, 0, "Got no mozvisualresize events in content");
-  visScrEvtContent.unregister();
-  is(visScrEvtContent.count, 0, "Got no mozvisualscroll events in content");
-}
-
-waitUntilApzStable()
-.then(runContinuation(test))
-.then(subtestDone);
-
-  </script>
-</head>
-<body>
-  Here is some text to stare at as the test runs. It serves no functional
-  purpose, but gives you an idea of the zoom level. It's harder to tell what
-  the zoom level is when the page is just solid white.
-</body>
-</html>
deleted file mode 100644
--- a/gfx/layers/apz/test/mochitest/helper_onetouchpinch_nested.html
+++ /dev/null
@@ -1,109 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-  <meta charset="utf-8">
-  <meta name="viewport" content="width=device-width">
-  <title>One-touch pinch zooming while on a non-root scroller</title>
-  <script type="application/javascript" src="apz_test_native_event_utils.js"></script>
-  <script type="application/javascript" src="apz_test_utils.js"></script>
-  <script type="application/javascript" src="/tests/SimpleTest/paint_listener.js"></script>
-  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
-  <script type="application/javascript">
-
-function* test_onetouchpinch(testDriver) {
-  // layerize the scroller so it gets an APZC and GestureEventListener
-  var scroller = document.getElementById("scroller");
-  SpecialPowers.getDOMWindowUtils(window).setDisplayPortForElement(0, 0, 400, 1000, scroller, 1);
-  yield waitForApzFlushedRepaints(testDriver);
-
-  ok(isLayerized("scroller"), "scroller has been successfully layerized");
-
-  var initial_resolution = getResolution();
-  ok(initial_resolution > 0,
-      "The initial_resolution is " + initial_resolution + ", which is some sane value");
-
-  // This listener will trigger the test to continue once APZ is done with
-  // processing the scroll.
-  SpecialPowers.Services.obs.addObserver(testDriver, "APZ:TransformEnd");
-
-  function translateY(point, dy) {
-    return {x: point.x, y: point.y + dy};
-  }
-
-  var zoom_point = centerOf(scroller);
-  var zoom_in = [
-      [ zoom_point ],
-      [ null ],
-      [ zoom_point ],
-      [ translateY(zoom_point, 5) ],
-      [ translateY(zoom_point, 10) ],
-      [ translateY(zoom_point, 15) ],
-      [ translateY(zoom_point, 20) ],
-      [ translateY(zoom_point, 25) ],
-  ];
-
-  var touchIds = [0];
-  yield* synthesizeNativeTouchSequences(scroller, zoom_in, null, touchIds);
-
-  // Wait for the APZ:TransformEnd to be fired after touch events are processed.
-  yield true;
-
-  // We get here once the APZ:TransformEnd has fired, so we don't need that
-  // observer any more.
-  SpecialPowers.Services.obs.removeObserver(testDriver, "APZ:TransformEnd");
-
-  // Flush state and get the resolution we're at now
-  yield waitForApzFlushedRepaints(testDriver);
-  let final_resolution = getResolution();
-  ok(final_resolution > initial_resolution, "The final resolution (" + final_resolution + ") is greater after zooming in");
-
-  // Also check that the scroller didn't get scrolled.
-  is(scroller.scrollTop, 0, "scroller didn't y-scroll");
-  is(scroller.scrollLeft, 0, "scroller didn't x-scroll");
-}
-
-function* test(testDriver) {
-  // Run the test with the scrollable div
-  yield* test_onetouchpinch(testDriver);
-  dump("Wrapping scroller in fixed-pos div...\n");
-  // Now wrap the scrollable div inside a fixed-pos div
-  var fixedElement = document.createElement("div");
-  fixedElement.id = "fixed";
-  document.body.appendChild(fixedElement);
-  fixedElement.appendChild(document.getElementById("scroller"));
-  dump("Done wrapping scroller in fixed-pos div.\n");
-  // Now run the test again, with the scrollable div inside a fixed-pos div
-  yield* test_onetouchpinch(testDriver);
-}
-
-waitUntilApzStable()
-.then(runContinuation(test))
-.then(subtestDone);
-
-  </script>
-  <style>
-    #scroller {
-        width: 300px;
-        height: 300px;
-        overflow: scroll;
-    }
-
-    #fixed {
-        background-color: green;
-        position: fixed;
-        width: 300px;
-        height: 300px;
-        left: 100px;
-        top: 100px;
-    }
-  </style>
-</head>
-<body>
-  Here is some text outside the scrollable div.
-  <div id="scroller">
-   Here is some text inside the scrollable div.
-   <div style="height: 2000px">This div actually makes it overflow.</div>
-  </div>
-  <div style="height: 2000px">This div makes the body scrollable.</div>
-</body>
-</html>
--- a/gfx/layers/apz/test/mochitest/test_group_zoom.html
+++ b/gfx/layers/apz/test/mochitest/test_group_zoom.html
@@ -40,35 +40,23 @@ var prefs = [
 // Increase the tap timeouts so the double-tap is still detected in case of
 // random delays during testing.
 var doubletap_prefs = [
   ...prefs,
   ["ui.click_hold_context_menus.delay", 10000],
   ["apz.max_tap_time", 10000],
 ];
 
-// Increase the tap timeouts so the one-touch-pinch gesture is still detected
-// in case of random delays during testing. Also ensure that the feature is
-// actually enabled (which it should be by default, but it's good to be safe).
-var onetouchpinch_prefs = [
-  ...prefs,
-  ["apz.one_touch_pinch.enabled", true],
-  ["ui.click_hold_context_menus.delay", 10000],
-  ["apz.max_tap_time", 10000],
-];
-
 var subtests = [
   {"file": "helper_bug1280013.html", "prefs": prefs},
   {"file": "helper_basic_zoom.html", "prefs": prefs},
-  {"file": "helper_basic_onetouchpinch.html", "prefs": onetouchpinch_prefs},
   {"file": "helper_zoom_prevented.html", "prefs": prefs},
   {"file": "helper_zoomed_pan.html", "prefs": prefs},
   {"file": "helper_fixed_position_scroll_hittest.html", "prefs": prefs},
   {"file": "helper_basic_doubletap_zoom.html", "prefs": doubletap_prefs},
-  {"file": "helper_onetouchpinch_nested.html", "prefs": onetouchpinch_prefs},
 ];
 
 if (isApzEnabled()) {
   // This has a lot of subtests, and Android emulators are slow.
   SimpleTest.requestLongerTimeout(2);
   SimpleTest.waitForExplicitFinish();
   window.onload = function() {
     runSubtestsSeriallyInFreshWindows(subtests)