Bug 736729 - Fix display-port on first paint and page-size changes. r=kats
authorChris Lord <chrislord.net@gmail.com>
Wed, 21 Mar 2012 14:44:33 +0000
changeset 89925 d7aa02178f7fc3d2001f8149b8a1f78780bee3dc
parent 89924 6a2c57fa8edf34d03c7f65915d3abc187a792381
child 89926 fd8273e5c1b2a5da69b65c660f8467a0649efdf4
push id22300
push usermak77@bonardo.net
push dateThu, 22 Mar 2012 12:08:29 +0000
treeherdermozilla-central@b622d692b8ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs736729
milestone14.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 736729 - Fix display-port on first paint and page-size changes. r=kats Previously, we only set the display-port in response to a page scrolling itself, or when we adjust the viewport. This meant that the display-port could be incorrect if a viewport adjustment was sent after a page-size changed, but before a render had completed. Similarly, we were not updating the display-port when the viewport of a foreground document that hadn't been displayed yet was changing. This would cause the first-paint to have an incorrect (and often too small) display port, which wouldn't be corrected until the page was scrolled.
mobile/android/base/gfx/GeckoLayerClient.java
mobile/android/chrome/content/browser.js
--- a/mobile/android/base/gfx/GeckoLayerClient.java
+++ b/mobile/android/base/gfx/GeckoLayerClient.java
@@ -104,16 +104,17 @@ public class GeckoLayerClient implements
         LayerView view = layerController.getView();
 
         mLayerController = layerController;
 
         mRootLayer = new VirtualLayer(new IntSize(view.getWidth(), view.getHeight()));
         mLayerRenderer = new LayerRenderer(view);
 
         GeckoAppShell.registerGeckoEventListener("Viewport:Update", this);
+        GeckoAppShell.registerGeckoEventListener("Viewport:PageSize", this);
         GeckoAppShell.registerGeckoEventListener("Viewport:CalculateDisplayPort", this);
         GeckoAppShell.registerGeckoEventListener("Checkerboard:Toggle", this);
 
         view.setListener(this);
         view.setLayerRenderer(mLayerRenderer);
         layerController.setRoot(mRootLayer);
 
         sendResizeEventIfNecessary(true);
@@ -237,35 +238,65 @@ public class GeckoLayerClient implements
 
         viewportMetrics.setViewport(viewportMetrics.getClampedViewport());
 
         mDisplayPort = calculateDisplayPort(mLayerController.getViewportMetrics());
         GeckoAppShell.sendEventToGecko(GeckoEvent.createViewportEvent(viewportMetrics, mDisplayPort));
         mGeckoViewport = viewportMetrics;
     }
 
+    /**
+     * The different types of Viewport messages handled. All viewport events
+     * expect a display-port to be returned, but can handle one not being
+     * returned.
+     */
+    private enum ViewportMessageType {
+        UPDATE,       // The viewport has changed and should be entirely updated
+        PAGE_SIZE     // The viewport's page-size has changed
+    }
+
+    /** Viewport message handler. */
+    private void handleViewportMessage(JSONObject message, ViewportMessageType type) throws JSONException {
+        ViewportMetrics messageMetrics = new ViewportMetrics(message);
+        synchronized (mLayerController) {
+            final ViewportMetrics newMetrics;
+            ImmutableViewportMetrics oldMetrics = mLayerController.getViewportMetrics();
+
+            switch (type) {
+            default:
+            case UPDATE:
+                newMetrics = messageMetrics;
+                // Keep the old viewport size
+                newMetrics.setSize(oldMetrics.getSize());
+                mLayerController.abortPanZoomAnimation();
+                break;
+            case PAGE_SIZE:
+                newMetrics = new ViewportMetrics(oldMetrics);
+                newMetrics.setPageSize(messageMetrics.getPageSize());
+                break;
+            }
+
+            mLayerController.post(new Runnable() {
+                public void run() {
+                    mGeckoViewport = newMetrics;
+                }
+            });
+            mLayerController.setViewportMetrics(newMetrics);
+            mDisplayPort = calculateDisplayPort(mLayerController.getViewportMetrics());
+        }
+        mReturnDisplayPort = mDisplayPort;
+    }
+
     /** Implementation of GeckoEventResponder/GeckoEventListener. */
     public void handleMessage(String event, JSONObject message) {
         try {
             if ("Viewport:Update".equals(event)) {
-                final ViewportMetrics newMetrics = new ViewportMetrics(message);
-                synchronized (mLayerController) {
-                    // keep the old viewport size, but update everything else
-                    ImmutableViewportMetrics oldMetrics = mLayerController.getViewportMetrics();
-                    newMetrics.setSize(oldMetrics.getSize());
-                    mLayerController.post(new Runnable() {
-                        public void run() {
-                            mGeckoViewport = newMetrics;
-                        }
-                    });
-                    mLayerController.setViewportMetrics(newMetrics);
-                    mLayerController.abortPanZoomAnimation();
-                    mDisplayPort = calculateDisplayPort(mLayerController.getViewportMetrics());
-                    mReturnDisplayPort = mDisplayPort;
-                }
+                handleViewportMessage(message, ViewportMessageType.UPDATE);
+            } else if ("Viewport:PageSize".equals(event)) {
+                handleViewportMessage(message, ViewportMessageType.PAGE_SIZE);
             } else if ("Viewport:CalculateDisplayPort".equals(event)) {
                 ImmutableViewportMetrics newMetrics = new ImmutableViewportMetrics(new ViewportMetrics(message));
                 mReturnDisplayPort = calculateDisplayPort(newMetrics);
             } else if ("Checkerboard:Toggle".equals(event)) {
                 try {
                     boolean showChecks = message.getBoolean("value");
                     mLayerController.setCheckerboardShowChecks(showChecks);
                     Log.i(LOGTAG, "Showing checks: " + showChecks);
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -1510,16 +1510,17 @@ Tab.prototype = {
     this.browser.sessionHistory.addSHistoryListener(this);
 
     this.browser.addEventListener("DOMContentLoaded", this, true);
     this.browser.addEventListener("DOMLinkAdded", this, true);
     this.browser.addEventListener("DOMTitleChanged", this, true);
     this.browser.addEventListener("DOMWindowClose", this, true);
     this.browser.addEventListener("DOMWillOpenModalDialog", this, true);
     this.browser.addEventListener("scroll", this, true);
+    this.browser.addEventListener("MozScrolledAreaChanged", this, true);
     this.browser.addEventListener("PluginClickToPlay", this, true);
     this.browser.addEventListener("pagehide", this, true);
     this.browser.addEventListener("pageshow", this, true);
 
     Services.obs.addObserver(this, "before-first-paint", false);
 
     if (!aParams.delayLoad) {
       let flags = "flags" in aParams ? aParams.flags : Ci.nsIWebNavigation.LOAD_FLAGS_NONE;
@@ -1557,16 +1558,17 @@ Tab.prototype = {
     this.browser.removeProgressListener(this);
     this.browser.removeEventListener("DOMContentLoaded", this, true);
     this.browser.removeEventListener("DOMLinkAdded", this, true);
     this.browser.removeEventListener("DOMTitleChanged", this, true);
     this.browser.removeEventListener("DOMWindowClose", this, true);
     this.browser.removeEventListener("DOMWillOpenModalDialog", this, true);
     this.browser.removeEventListener("scroll", this, true);
     this.browser.removeEventListener("PluginClickToPlay", this, true);
+    this.browser.removeEventListener("MozScrolledAreaChanged", this, true);
     this.browser.removeEventListener("pagehide", this, true);
     this.browser.removeEventListener("pageshow", this, true);
 
     Services.obs.removeObserver(this, "before-first-paint");
 
     // 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;
@@ -1684,27 +1686,26 @@ Tab.prototype = {
         viewport.pageWidth = pageWidth;
         viewport.pageHeight = pageHeight;
       }
     }
 
     return viewport;
   },
 
-  sendViewportUpdate: function() {
+  sendViewportUpdate: function(aPageSizeUpdate) {
     let message;
-    if (BrowserApp.selectedTab == this) {
-      // for foreground tabs, send the viewport update unless the document
-      // displayed is different from the content document
-      if (!BrowserApp.isBrowserContentDocumentDisplayed())
-        return;
+    // for foreground tabs, send the viewport update unless the document
+    // displayed is different from the content document. In that case, just
+    // calculate the display port.
+    if (BrowserApp.selectedTab == this && BrowserApp.isBrowserContentDocumentDisplayed()) {
       message = this.getViewport();
-      message.type = "Viewport:Update";
+      message.type = aPageSizeUpdate ? "Viewport:PageSize" : "Viewport:Update";
     } else {
-      // for bcakground tabs, request a new display port calculation, so that
+      // for background tabs, request a new display port calculation, so that
       // when we do switch to that tab, we have the correct display port and
       // don't need to draw twice (once to allow the first-paint viewport to
       // get to java, and again once java figures out the display port).
       message = this.getViewport();
       message.type = "Viewport:CalculateDisplayPort";
     }
     let displayPort = sendMessageToJava({ gecko: message });
     if (displayPort != null)
@@ -1854,16 +1855,27 @@ Tab.prototype = {
       case "scroll": {
         let win = this.browser.contentWindow;
         if (this.userScrollPos.x != win.scrollX || this.userScrollPos.y != win.scrollY) {
           this.sendViewportUpdate();
         }
         break;
       }
 
+      case "MozScrolledAreaChanged": {
+        // This event is only fired for root scroll frames, and only when the
+        // scrolled area has actually changed, so no need to check for that.
+        // Just make sure it's the event for the correct root scroll frame.
+        if (aEvent.originalTarget != this.browser.contentDocument)
+          return;
+
+        this.sendViewportUpdate(true);
+        break;
+      }
+
       case "PluginClickToPlay": {
         // Keep track of the number of plugins to know whether or not to show
         // the hidden plugins doorhanger
         this._pluginCount++;
 
         let plugin = aEvent.target;
         let overlay = plugin.ownerDocument.getAnonymousElementByAttribute(plugin, "class", "mainBox");
         if (!overlay)
@@ -2189,21 +2201,22 @@ Tab.prototype = {
         let contentDocument = aSubject;
         if (contentDocument == this.browser.contentDocument) {
           // reset CSS viewport and zoom to default on new page
           this.setBrowserSize(kDefaultCSSViewportWidth, kDefaultCSSViewportHeight);
           this.setResolution(gScreenWidth / this.browserWidth, false);
           // and then use the metadata to figure out how it needs to be updated
           ViewportHandler.updateMetadata(this);
 
-          // The document element must have a display port on it whenever we are about to
-          // paint. This is the point just before the first paint, so we set the display port
-          // to a default value here. Once Java is aware of this document it will overwrite
-          // it with a better-calculated display port.
-          this.setDisplayPort(0, 0, {left: 0, top: 0, right: gScreenWidth, bottom: gScreenHeight });
+          // If we draw without a display-port, things can go wrong. While it's
+          // almost certain a display-port has been set via the
+          // MozScrolledAreaChanged event, make sure by sending a viewport
+          // update here. As it's the first paint, this will end up being a
+          // display-port request only.
+          this.sendViewportUpdate();
 
           BrowserApp.displayedDocumentChanged();
           this.contentDocumentIsDisplayed = true;
         }
         break;
     }
   },