Bug 732564 - Modify the Viewport:Update event to take a viewport and synchronously update Java with it. r=Cwiiis
authorKartikaya Gupta <kgupta@mozilla.com>
Mon, 12 Mar 2012 12:03:38 -0400
changeset 89325 8bcc8f581217f544a25b008863e3520da7ee53f6
parent 89324 a5980ce21d40d091ed70e53809cf38e30d56abf7
child 89326 212f97fe62cb8f6a31f40c34aec5a50b9511250e
push id22242
push userkgupta@mozilla.com
push dateWed, 14 Mar 2012 15:19:09 +0000
treeherdermozilla-central@936ef50fa498 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersCwiiis
bugs732564
milestone13.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 732564 - Modify the Viewport:Update event to take a viewport and synchronously update Java with it. r=Cwiiis
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
@@ -71,30 +71,20 @@ public class GeckoLayerClient implements
     private IntSize mBufferSize;
     private Rect mDisplayPortMargins;
 
     private VirtualLayer mRootLayer;
 
     /* The viewport that Gecko is currently displaying. */
     private ViewportMetrics mGeckoViewport;
 
-    /* The viewport that Gecko will display when drawing is finished */
-    private ViewportMetrics mNewGeckoViewport;
-
     private boolean mViewportSizeChanged;
     private boolean mIgnorePaintsPendingViewportSizeChange;
     private boolean mFirstPaint = true;
 
-    // mUpdateViewportOnEndDraw is used to indicate that we received a
-    // viewport update notification while drawing. therefore, when the
-    // draw finishes, we need to update the entire viewport rather than
-    // just the page size. this boolean should always be accessed from
-    // inside a transaction, so no synchronization is needed.
-    private boolean mUpdateViewportOnEndDraw;
-
     private String mLastCheckerboardColor;
 
     /* Used by robocop for testing purposes */
     private DrawListener mDrawListener;
 
     /* Used as a temporary ViewTransform by getViewTransform */
     private ViewTransform mCurrentViewTransform;
 
@@ -142,54 +132,34 @@ public class GeckoLayerClient implements
         // If we've changed surface types, cancel this draw
         if (initializeVirtualLayer()) {
             Log.e(LOGTAG, "### Cancelling draw due to virtual layer initialization");
             return false;
         }
 
         try {
             JSONObject viewportObject = new JSONObject(metadata);
-            mNewGeckoViewport = new ViewportMetrics(viewportObject);
+            mGeckoViewport = new ViewportMetrics(viewportObject);
         } catch (JSONException e) {
             Log.e(LOGTAG, "Aborting draw, bad viewport description: " + metadata);
             return false;
         }
 
         if (mBufferSize.width != width || mBufferSize.height != height) {
             mBufferSize = new IntSize(width, height);
         }
 
         return true;
     }
 
     /** This function is invoked by Gecko via JNI; be careful when modifying signature. */
     public void endDrawing() {
         synchronized (mLayerController) {
-            // save and restore the viewport size stored in java; never let the
-            // JS-side viewport dimensions override the java-side ones because
-            // java is the One True Source of this information, and allowing JS
-            // to override can lead to race conditions where this data gets clobbered.
-            FloatSize viewportSize = mLayerController.getViewportSize();
-            mGeckoViewport = mNewGeckoViewport;
-            mGeckoViewport.setSize(viewportSize);
-
             RectF position = mGeckoViewport.getViewport();
             mRootLayer.setPositionAndResolution(RectUtils.round(position), mGeckoViewport.getZoomFactor());
-
-            if (mUpdateViewportOnEndDraw) {
-                mLayerController.setViewportMetrics(mGeckoViewport);
-                mLayerController.abortPanZoomAnimation();
-                mUpdateViewportOnEndDraw = false;
-            } else {
-                // Don't adjust page size when zooming unless zoom levels are
-                // approximately equal.
-                if (FloatUtils.fuzzyEquals(mLayerController.getZoomFactor(),
-                        mGeckoViewport.getZoomFactor()))
-                    mLayerController.setPageSize(mGeckoViewport.getPageSize());
-            }
         }
 
         Log.i(LOGTAG, "zerdatime " + SystemClock.uptimeMillis() + " - endDrawing");
         /* Used by robocop for testing purposes */
         if (mDrawListener != null) {
             mDrawListener.drawFinished();
         }
     }
@@ -273,18 +243,30 @@ public class GeckoLayerClient implements
             mViewportSizeChanged = false;
             GeckoAppShell.viewSizeChanged();
         }
     }
 
     /** Implementation of GeckoEventResponder/GeckoEventListener. */
     public void handleMessage(String event, JSONObject message) {
         if ("Viewport:Update".equals(event)) {
-            mUpdateViewportOnEndDraw = true;
             mIgnorePaintsPendingViewportSizeChange = false;
+
+            try {
+                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.setViewportMetrics(newMetrics);
+                    mLayerController.abortPanZoomAnimation();
+                }
+            } catch (JSONException e) {
+                Log.e(LOGTAG, "Unable to create viewport metrics in " + event + " handler", e);
+            }
         }
     }
 
     /** Implementation of GeckoEventResponder. */
     public String getResponse() {
         // We are responding to the events handled in handleMessage() above with
         // the display port margins we want. Note that all messages we are currently
         // handling (Viewport:Update) require this response, so we can just return
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -1727,21 +1727,19 @@ Tab.prototype = {
     }
 
     return viewport;
   },
 
   sendViewportUpdate: function() {
     if (BrowserApp.selectedTab != this)
       return;
-    let displayPortMargins = sendMessageToJava({
-      gecko: {
-        type: "Viewport:Update"
-      }
-    });
+    let message = this.getViewport();
+    message.type = "Viewport:Update";
+    let displayPortMargins = sendMessageToJava({ gecko: message });
     if (displayPortMargins != null)
       this.refreshDisplayPort(JSON.parse(displayPortMargins));
   },
 
   handleEvent: function(aEvent) {
     switch (aEvent.type) {
       case "DOMContentLoaded": {
         let target = aEvent.originalTarget;