Bug 878935: Pause painting while reflow-on-zoom is in progress to provide a better user experience. [r=kats]
authorScott Johnson <sjohnson@mozilla.com>
Tue, 01 Oct 2013 14:52:13 -0500
changeset 164313 6a5549d97d8d60effeb0696979af650e0b6a5eb7
parent 164312 22da748201eb334b46d5ea870230a770be3f5e2a
child 164314 d7599a4f3e17acc69bb3cf03a0cd4fe4e4b466cb
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs878935
milestone27.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 878935: Pause painting while reflow-on-zoom is in progress to provide a better user experience. [r=kats]
docshell/base/nsIMarkupDocumentViewer.idl
layout/base/nsDocumentViewer.cpp
layout/base/nsIPresShell.h
layout/base/nsPresShell.cpp
layout/base/nsPresShell.h
mobile/android/chrome/content/browser.js
--- a/docshell/base/nsIMarkupDocumentViewer.idl
+++ b/docshell/base/nsIMarkupDocumentViewer.idl
@@ -132,16 +132,28 @@ interface nsIMarkupDocumentViewer : nsIS
    * Set the maximum line width for the document.
    * NOTE: This will generate a reflow!
    *
    * @param maxLineWidth The maximum width of any line boxes on the page,
    *        in CSS pixels.
    */
   void changeMaxLineBoxWidth(in int32_t maxLineBoxWidth);
 
+  /**
+   * Instruct the refresh driver to discontinue painting until further
+   * notice.
+   */
+  void pausePainting();
+
+  /**
+   * Instruct the refresh driver to resume painting after a previous call to
+   * pausePainting().
+   */
+  void resumePainting();
+
   /*
    * Render the document as if being viewed on a device with the specified
    * media type. This will cause a reflow.
    *
    * @param mediaType The media type to be emulated
    */
   void emulateMedium(in AString aMediaType);
 
--- a/layout/base/nsDocumentViewer.cpp
+++ b/layout/base/nsDocumentViewer.cpp
@@ -2653,16 +2653,27 @@ nsDocumentViewer::CallChildren(CallChild
 }
 
 struct LineBoxInfo
 {
   nscoord mMaxLineBoxWidth;
 };
 
 static void
+ChangeChildPaintingEnabled(nsIMarkupDocumentViewer* aChild, void* aClosure)
+{
+  bool* enablePainting = (bool*) aClosure;
+  if (*enablePainting) {
+    aChild->ResumePainting();
+  } else {
+    aChild->PausePainting();
+  }
+}
+
+static void
 ChangeChildMaxLineBoxWidth(nsIMarkupDocumentViewer* aChild, void* aClosure)
 {
   struct LineBoxInfo* lbi = (struct LineBoxInfo*) aClosure;
   aChild->ChangeMaxLineBoxWidth(lbi->mMaxLineBoxWidth);
 }
 
 struct ZoomInfo
 {
@@ -3259,17 +3270,46 @@ AppendChildSubtree(nsIMarkupDocumentView
 
 NS_IMETHODIMP nsDocumentViewer::AppendSubtree(nsTArray<nsCOMPtr<nsIMarkupDocumentViewer> >& aArray)
 {
   aArray.AppendElement(this);
   CallChildren(AppendChildSubtree, &aArray);
   return NS_OK;
 }
 
-NS_IMETHODIMP nsDocumentViewer::ChangeMaxLineBoxWidth(int32_t aMaxLineBoxWidth)
+nsresult
+nsDocumentViewer::PausePainting()
+{
+  bool enablePaint = false;
+  CallChildren(ChangeChildPaintingEnabled, &enablePaint);
+
+  nsIPresShell* presShell = GetPresShell();
+  if (presShell) {
+    presShell->FreezePainting();
+  }
+
+  return NS_OK;
+}
+
+nsresult
+nsDocumentViewer::ResumePainting()
+{
+  bool enablePaint = true;
+  CallChildren(ChangeChildPaintingEnabled, &enablePaint);
+
+  nsIPresShell* presShell = GetPresShell();
+  if (presShell) {
+    presShell->ThawPainting();
+  }
+
+  return NS_OK;
+}
+
+nsresult
+nsDocumentViewer::ChangeMaxLineBoxWidth(int32_t aMaxLineBoxWidth)
 {
   // Change the max line box width for all children.
   struct LineBoxInfo lbi = { aMaxLineBoxWidth };
   CallChildren(ChangeChildMaxLineBoxWidth, &lbi);
 
   // Now, change our max line box width.
   // Convert to app units, since our input is in CSS pixels.
   nscoord mlbw = nsPresContext::CSSPixelsToAppUnits(aMaxLineBoxWidth);
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -816,16 +816,28 @@ public:
   /**
    * Called to find out if painting is suppressed for this presshell.  If it is suppressd,
    * we don't allow the painting of any layer but the background, and we don't
    * recur into our children.
    */
   bool IsPaintingSuppressed() const { return mPaintingSuppressed; }
 
   /**
+   * Resume painting by thawing the refresh driver of this and all parent
+   * presentations.
+   */
+  virtual void FreezePainting() = 0;
+
+  /**
+   * Resume painting by thawing the refresh driver of this and all parent
+   * presentations.
+   */
+  virtual void ThawPainting() = 0;
+
+  /**
    * Unsuppress painting.
    */
   virtual NS_HIDDEN_(void) UnsuppressPainting() = 0;
 
   /**
    * Called to disable nsITheme support in a specific presshell.
    */
   void DisableThemeSupport()
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -9704,8 +9704,32 @@ nsIPresShell::SetMaxLineBoxWidth(nscoord
   NS_ASSERTION(aMaxLineBoxWidth >= 0, "attempting to set max line box width to a negative value");
 
   if (mMaxLineBoxWidth != aMaxLineBoxWidth) {
     mMaxLineBoxWidth = aMaxLineBoxWidth;
     mReflowOnZoomPending = true;
     FrameNeedsReflow(GetRootFrame(), eResize, NS_FRAME_HAS_DIRTY_CHILDREN);
   }
 }
+
+void
+PresShell::FreezePainting()
+{
+  // We want to freeze painting all the way up the presentation hierarchy.
+  nsCOMPtr<nsIPresShell> parent = GetParentPresShell();
+  if (parent) {
+    parent->FreezePainting();
+  }
+
+  GetPresContext()->RefreshDriver()->Freeze();
+}
+
+void
+PresShell::ThawPainting()
+{
+  // We want to thaw painting all the way up the presentation hierarchy.
+  nsCOMPtr<nsIPresShell> parent = GetParentPresShell();
+  if (parent) {
+    parent->ThawPainting();
+  }
+
+  GetPresContext()->RefreshDriver()->Thaw();
+}
--- a/layout/base/nsPresShell.h
+++ b/layout/base/nsPresShell.h
@@ -705,16 +705,19 @@ protected:
   bool ScheduleReflowOffTimer();
 
   // Widget notificiations
   virtual void WindowSizeMoveDone() MOZ_OVERRIDE;
   virtual void SysColorChanged() MOZ_OVERRIDE { mPresContext->SysColorChanged(); }
   virtual void ThemeChanged() MOZ_OVERRIDE { mPresContext->ThemeChanged(); }
   virtual void BackingScaleFactorChanged() MOZ_OVERRIDE { mPresContext->UIResolutionChanged(); }
 
+  virtual void FreezePainting() MOZ_OVERRIDE;
+  virtual void ThawPainting() MOZ_OVERRIDE;
+
   void UpdateImageVisibility();
 
   nsRevocableEventPtr<nsRunnableMethod<PresShell> > mUpdateImageVisibilityEvent;
 
   void ClearVisibleImagesList();
   static void ClearImageVisibilityVisited(nsView* aView, bool aClear);
   static void MarkImagesInListVisible(const nsDisplayList& aList);
 
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -167,16 +167,21 @@ function doChangeMaxLineBoxWidth(aWidth)
   if (BrowserApp.selectedTab._mReflozPoint) {
     range = BrowserApp.selectedTab._mReflozPoint.range;
   }
 
   docViewer.changeMaxLineBoxWidth(aWidth);
 
   if (range) {
     BrowserEventHandler._zoomInAndSnapToRange(range);
+  } else {
+    // In this case, we actually didn't zoom into a specific range. It probably
+    // happened from a page load reflow-on-zoom event, so we need to make sure
+    // painting is re-enabled.
+    BrowserApp.selectedTab.clearReflowOnZoomPendingActions();
   }
 }
 
 function fuzzyEquals(a, b) {
   return (Math.abs(a - b) < 1e-6);
 }
 
 /**
@@ -2685,16 +2690,17 @@ Tab.prototype = {
     this.browser.addEventListener("scroll", this, true);
     this.browser.addEventListener("MozScrolledAreaChanged", this, true);
     // Note that the XBL binding is untrusted
     this.browser.addEventListener("PluginBindingAttached", this, true, true);
     this.browser.addEventListener("pageshow", this, true);
     this.browser.addEventListener("MozApplicationManifest", this, true);
 
     Services.obs.addObserver(this, "before-first-paint", false);
+    Services.obs.addObserver(this, "after-viewport-change", false);
     Services.prefs.addObserver("browser.ui.zoom.force-user-scalable", this, false);
 
     if (aParams.delayLoad) {
       // If this is a zombie tab, attach restore data so the tab will be
       // restored when selected
       this.browser.__SS_data = {
         entries: [{
           url: aURL,
@@ -2747,31 +2753,68 @@ Tab.prototype = {
     // We only use the font.size.inflation.minTwips preference because this is
     // the only one that is controlled by the user-interface in the 'Settings'
     // menu. Thus, if font.size.inflation.emPerLine is changed, this does not
     // effect reflow-on-zoom.
     let minFontSize = convertFromTwipsToPx(Services.prefs.getIntPref("font.size.inflation.minTwips"));
     return minFontSize / this.getInflatedFontSizeFor(aElement);
   },
 
+  clearReflowOnZoomPendingActions: function() {
+    // Reflow was completed, so now re-enable painting.
+    let webNav = BrowserApp.selectedTab.window.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIWebNavigation);
+    let docShell = webNav.QueryInterface(Ci.nsIDocShell);
+    let docViewer = docShell.contentViewer.QueryInterface(Ci.nsIMarkupDocumentViewer);
+    docViewer.resumePainting();
+
+    BrowserApp.selectedTab._mReflozPositioned = false;
+  },
+
+  /**
+   * Reflow on zoom consists of a few different sub-operations:
+   *
+   * 1. When a double-tap event is seen, we verify that the correct preferences
+   *    are enabled and perform the pre-position handling calculation. We also
+   *    signal that reflow-on-zoom should be performed at this time, and pause
+   *    painting.
+   * 2. During the next call to setViewport(), which is in the Tab prototype,
+   *    we detect that a call to changeMaxLineBoxWidth should be performed. If
+   *    we're zooming out, then the max line box width should be reset at this
+   *    time. Otherwise, we call performReflowOnZoom.
+   *   2a. PerformReflowOnZoom() and resetMaxLineBoxWidth() schedule a call to
+   *       doChangeMaxLineBoxWidth, based on a timeout specified in preferences.
+   * 3. doChangeMaxLineBoxWidth changes the line box width (which also
+   *    schedules a reflow event), and then calls _zoomInAndSnapToRange.
+   * 4. _zoomInAndSnapToRange performs the positioning of reflow-on-zoom and
+   *    then re-enables painting.
+   *
+   * Some of the events happen synchronously, while others happen asynchronously.
+   * The following is a rough sketch of the progression of events:
+   *
+   * double tap event seen -> onDoubleTap() -> ... asynchronous ...
+   *   -> setViewport() -> performReflowOnZoom() -> ... asynchronous ...
+   *   -> doChangeMaxLineBoxWidth() -> _zoomInAndSnapToRange()
+   *   -> ... asynchronous ... -> setViewport() -> Observe('after-viewport-change')
+   *   -> resumePainting()
+   */
   performReflowOnZoom: function(aViewport) {
-      let zoom = this._drawZoom ? this._drawZoom : aViewport.zoom;
-
-      let viewportWidth = gScreenWidth / zoom;
-      let reflozTimeout = Services.prefs.getIntPref("browser.zoom.reflowZoom.reflowTimeout");
-
-      if (gReflowPending) {
-        clearTimeout(gReflowPending);
-      }
-
-      // We add in a bit of fudge just so that the end characters
-      // don't accidentally get clipped. 15px is an arbitrary choice.
-      gReflowPending = setTimeout(doChangeMaxLineBoxWidth,
-                                  reflozTimeout,
-                                  viewportWidth - 15);
+    let zoom = this._drawZoom ? this._drawZoom : aViewport.zoom;
+
+    let viewportWidth = gScreenWidth / zoom;
+    let reflozTimeout = Services.prefs.getIntPref("browser.zoom.reflowZoom.reflowTimeout");
+
+    if (gReflowPending) {
+      clearTimeout(gReflowPending);
+    }
+
+    // We add in a bit of fudge just so that the end characters
+    // don't accidentally get clipped. 15px is an arbitrary choice.
+    gReflowPending = setTimeout(doChangeMaxLineBoxWidth,
+                                reflozTimeout,
+                                viewportWidth - 15);
   },
 
   /** 
    * Reloads the tab with the desktop mode setting.
    */
   reloadWithMode: function (aDesktopMode) {
     // Set desktop mode for tab and send change to Java
     if (this.desktopMode != aDesktopMode) {
@@ -2833,16 +2876,17 @@ Tab.prototype = {
     this.browser.removeEventListener("blur", this, true);
     this.browser.removeEventListener("scroll", this, true);
     this.browser.removeEventListener("MozScrolledAreaChanged", this, true);
     this.browser.removeEventListener("PluginBindingAttached", this, true);
     this.browser.removeEventListener("pageshow", this, true);
     this.browser.removeEventListener("MozApplicationManifest", this, true);
 
     Services.obs.removeObserver(this, "before-first-paint");
+    Services.obs.removeObserver(this, "after-viewport-change");
     Services.prefs.removeObserver("browser.ui.zoom.force-user-scalable", this);
 
     // Make sure the previously selected panel remains selected. The selected panel of a deck is
     // not stable when panels are removed.
     let selectedPanel = BrowserApp.deck.selectedPanel;
     BrowserApp.deck.removeChild(this.browser);
     BrowserApp.deck.selectedPanel = selectedPanel;
 
@@ -3130,16 +3174,17 @@ Tab.prototype = {
       // because we are pinch-zooming to zoom out.
       BrowserEventHandler.resetMaxLineBoxWidth();
       BrowserApp.selectedTab.reflozPinchSeen = false;
     } else if (BrowserApp.selectedTab.reflozPinchSeen &&
                isZooming) {
       // In this case, the user pinch-zoomed in, so we don't want to
       // preserve position as we would with reflow-on-zoom.
       BrowserApp.selectedTab.probablyNeedRefloz = false;
+      BrowserApp.selectedTab.clearReflowOnZoomPendingActions();
       BrowserApp.selectedTab._mReflozPoint = null;
     }
 
     if (isZooming &&
         BrowserEventHandler.mReflozPref &&
         BrowserApp.selectedTab._mReflozPoint &&
         BrowserApp.selectedTab.probablyNeedRefloz) {
       BrowserApp.selectedTab.performReflowOnZoom(aViewport);
@@ -4040,16 +4085,21 @@ Tab.prototype = {
 
         if (rzEnabled && rzPl) {
           // Retrieve the viewport width and adjust the max line box width
           // accordingly.
           let vp = BrowserApp.selectedTab.getViewport();
           BrowserApp.selectedTab.performReflowOnZoom(vp);
         }
         break;
+      case "after-viewport-change":
+        if (BrowserApp.selectedTab._mReflozPositioned) {
+          BrowserApp.selectedTab.clearReflowOnZoomPendingActions();
+        }
+        break;
       case "nsPref:changed":
         if (aData == "browser.ui.zoom.force-user-scalable")
           ViewportHandler.updateMetadata(this, false);
         break;
     }
   },
 
   set readerEnabled(isReaderEnabled) {
@@ -4350,23 +4400,33 @@ var BrowserEventHandler = {
     let element = ElementTouchHelper.anyElementFromPoint(data.x, data.y);
 
     // We only want to do this if reflow-on-zoom is enabled, we don't already
     // have a reflow-on-zoom event pending, and the element upon which the user
     // double-tapped isn't of a type we want to avoid reflow-on-zoom.
     if (BrowserEventHandler.mReflozPref &&
        !BrowserApp.selectedTab._mReflozPoint &&
        !this._shouldSuppressReflowOnZoom(element)) {
-     let data = JSON.parse(aData);
-     let zoomPointX = data.x;
-     let zoomPointY = data.y;
-
-     BrowserApp.selectedTab._mReflozPoint = { x: zoomPointX, y: zoomPointY,
-       range: BrowserApp.selectedBrowser.contentDocument.caretPositionFromPoint(zoomPointX, zoomPointY) };
-       BrowserApp.selectedTab.probablyNeedRefloz = true;
+
+      // See comment above performReflowOnZoom() for a detailed description of
+      // the events happening in the reflow-on-zoom operation.
+      let data = JSON.parse(aData);
+      let zoomPointX = data.x;
+      let zoomPointY = data.y;
+
+      BrowserApp.selectedTab._mReflozPoint = { x: zoomPointX, y: zoomPointY,
+        range: BrowserApp.selectedBrowser.contentDocument.caretPositionFromPoint(zoomPointX, zoomPointY) };
+
+      // Before we perform a reflow on zoom, let's disable painting.
+      let webNav = BrowserApp.selectedTab.window.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIWebNavigation);
+      let docShell = webNav.QueryInterface(Ci.nsIDocShell);
+      let docViewer = docShell.contentViewer.QueryInterface(Ci.nsIMarkupDocumentViewer);
+      docViewer.pausePainting();
+
+      BrowserApp.selectedTab.probablyNeedRefloz = true;
     }
 
     if (!element) {
       this._zoomOut();
       return;
     }
 
     while (element && !this._shouldZoomToElement(element))
@@ -4456,21 +4516,17 @@ var BrowserEventHandler = {
     if (rect.w > viewport.cssWidth || rect.h > viewport.cssHeight) {
       BrowserEventHandler.resetMaxLineBoxWidth();
     }
 
     sendMessageToJava(rect);
   },
 
   _zoomInAndSnapToRange: function(aRange) {
-    if (!aRange) {
-      Cu.reportError("aRange is null in zoomInAndSnapToRange. Unable to maintain position.");
-      return;
-    }
-
+    // aRange is always non-null here, since a check happened previously.
     let viewport = BrowserApp.selectedTab.getViewport();
     let fudge = 15; // Add a bit of fudge.
     let boundingElement = aRange.offsetNode;
     while (!boundingElement.getBoundingClientRect && boundingElement.parentNode) {
       boundingElement = boundingElement.parentNode;
     }
 
     let rect = ElementTouchHelper.getBoundingContentRect(boundingElement);
@@ -4483,40 +4539,42 @@ var BrowserEventHandler = {
     // center the area of interest on the screen.
     let topPos = scrollTop + drRect.top - (viewport.cssHeight / 2.0);
 
     // Factor in the border and padding
     let boundingStyle = window.getComputedStyle(boundingElement);
     let leftAdjustment = parseInt(boundingStyle.paddingLeft) +
                          parseInt(boundingStyle.borderLeftWidth);
 
+    BrowserApp.selectedTab._mReflozPositioned = true;
+
     rect.type = "Browser:ZoomToRect";
     rect.x = Math.max(viewport.cssPageLeft, rect.x  - fudge + leftAdjustment);
     rect.y = Math.max(topPos, viewport.cssPageTop);
     rect.w = viewport.cssWidth;
     rect.h = viewport.cssHeight;
 
     sendMessageToJava(rect);
     BrowserApp.selectedTab._mReflozPoint = null;
-   },
-
-   onPinchFinish: function(aData) {
-     let data = {};
-     try {
-       data = JSON.parse(aData);
-     } catch(ex) {
-       console.log(ex);
-       return;
-     }
-
-     if (BrowserEventHandler.mReflozPref &&
-         data.zoomDelta < 0.0) {
-       BrowserEventHandler.resetMaxLineBoxWidth();
-     }
-   },
+  },
+
+  onPinchFinish: function(aData) {
+    let data = {};
+    try {
+      data = JSON.parse(aData);
+    } catch(ex) {
+      console.log(ex);
+      return;
+    }
+
+    if (BrowserEventHandler.mReflozPref &&
+        data.zoomDelta < 0.0) {
+      BrowserEventHandler.resetMaxLineBoxWidth();
+    }
+  },
 
   _shouldZoomToElement: function(aElement) {
     let win = aElement.ownerDocument.defaultView;
     if (win.getComputedStyle(aElement, null).display == "inline")
       return false;
     if (aElement instanceof Ci.nsIDOMHTMLLIElement)
       return false;
     if (aElement instanceof Ci.nsIDOMHTMLQuoteElement)