Bug 797615 - Guard against isBrowserContentDocumentDisplayed returning true prematurely. r=Cwiiis

Prior to this change, isBrowserContentDocumentDisplayed returned false
from the time that the isFirstPaint flag was set in layout to the time
that layout handed off the rendered document to the compositor. However
the way the function is used meant that it needs to return false until
the compositor actually composites the "first-paint" rendering,
otherwise other events can sneak in and run before the compositor. This
patch moves the tracking for the flag into GeckoLayerClient so that it
can be queried and modified synchronously from both the Gecko thread in
browser.js and the compositor thread in setFirstPaintViewport.

mobile/android/base/gfx/GeckoLayerClient.java

@@ -93,6 +93,15 @@ public class GeckoLayerClient implements LayerView.Listener, PanZoomTarget
     private final LayerMarginsAnimator mMarginsAnimator;
     private LayerView mView;
 
+    /* This flag is true from the time that browser.js detects a first-paint is about to start,
+     * to the time that we receive the first-paint composite notification from the compositor.
+     * Note that there is a small race condition with this; if there are two paints that both
+     * have the first-paint flag set, and the second paint happens concurrently with the
+     * composite for the first paint, then this flag may be set to true prematurely. Fixing this
+     * is possible but risky; see https://bugzilla.mozilla.org/show_bug.cgi?id=797615#c751
+     */
+    private volatile boolean mContentDocumentIsDisplayed;
+
     public GeckoLayerClient(Context context, LayerView view, EventDispatcher eventDispatcher) {
         // we can fill these in with dummy values because they are always written
         // to before being read
@@ -120,6 +129,7 @@ public class GeckoLayerClient implements LayerView.Listener, PanZoomTarget
         mMarginsAnimator = new LayerMarginsAnimator(this, view);
         mView = view;
         mView.setListener(this);
+        mContentDocumentIsDisplayed = true;
     }
 
     /** Attaches to root layer so that Gecko appears. */
@@ -413,6 +423,16 @@ public class GeckoLayerClient implements LayerView.Listener, PanZoomTarget
         }
     }
 
+    /* This is invoked by JNI on the gecko thread */
+    void contentDocumentChanged() {
+        mContentDocumentIsDisplayed = false;
+    }
+
+    /* This is invoked by JNI on the gecko thread */
+    boolean isContentDocumentDisplayed() {
+        return mContentDocumentIsDisplayed;
+    }
+
     // This is called on the Gecko thread to determine if we're still interested
     // in the update of this display-port to continue. We can return true here
     // to abort the current update and continue with any subsequent ones. This
@@ -579,6 +599,8 @@ public class GeckoLayerClient implements LayerView.Listener, PanZoomTarget
         }
         DisplayPortCalculator.resetPageState();
         mDrawTimingQueue.reset();
+
+        mContentDocumentIsDisplayed = true;
     }
 
     /** This function is invoked by Gecko via JNI; be careful when modifying signature.

mobile/android/chrome/content/browser.js

@@ -637,7 +637,7 @@ var BrowserApp = {
   // off to the compositor.
   isBrowserContentDocumentDisplayed: function() {
     try {
-      if (window.top.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils).isFirstPaint)
+      if (!getBridge().isContentDocumentDisplayed())
         return false;
     } catch (e) {
       return false;
@@ -649,8 +649,9 @@ var BrowserApp = {
     return tab.contentDocumentIsDisplayed;
   },
 
-  displayedDocumentChanged: function() {
+  contentDocumentChanged: function() {
     window.top.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils).isFirstPaint = true;
+    getBridge().contentDocumentChanged();
   },
 
   get tabs() {
@@ -677,7 +678,7 @@ var BrowserApp = {
     Tabs.touch(aTab);
     aTab.setActive(true);
     aTab.setResolution(aTab._zoom, true);
-    this.displayedDocumentChanged();
+    this.contentDocumentChanged();
     this.deck.selectedPanel = aTab.browser;
     // Focus the browser so that things like selection will be styled correctly.
     aTab.browser.focus();
@@ -1442,7 +1443,7 @@ var BrowserApp = {
         break;
 
       case "Viewport:Flush":
-        this.displayedDocumentChanged();
+        this.contentDocumentChanged();
         break;
 
       case "Passwords:Init": {
@@ -3840,7 +3841,7 @@ Tab.prototype = {
         let contentDocument = aSubject;
         if (contentDocument == this.browser.contentDocument) {
           if (BrowserApp.selectedTab == this) {
-            BrowserApp.displayedDocumentChanged();
+            BrowserApp.contentDocumentChanged();
           }
           this.contentDocumentIsDisplayed = true;
 

widget/android/AndroidBridge.cpp

@@ -2193,6 +2193,20 @@ NS_IMETHODIMP nsAndroidBridge::GetDisplayPort(bool aPageSizeUpdate, bool aIsBrow
     return NS_OK;
 }
 
+/* void displayedDocumentChanged(); */
+NS_IMETHODIMP nsAndroidBridge::ContentDocumentChanged()
+{
+    AndroidBridge::Bridge()->ContentDocumentChanged();
+    return NS_OK;
+}
+
+/* boolean isContentDocumentDisplayed(); */
+NS_IMETHODIMP nsAndroidBridge::IsContentDocumentDisplayed(bool *aRet)
+{
+    *aRet = AndroidBridge::Bridge()->IsContentDocumentDisplayed();
+    return NS_OK;
+}
+
 void
 AndroidBridge::NotifyDefaultPrevented(bool aDefaultPrevented)
 {
@@ -2697,6 +2711,26 @@ AndroidBridge::GetDisplayPort(bool aPageSizeUpdate, bool aIsBrowserContentDispla
     mLayerClient->GetDisplayPort(&jniFrame, aPageSizeUpdate, aIsBrowserContentDisplayed, tabId, metrics, displayPort);
 }
 
+void
+AndroidBridge::ContentDocumentChanged()
+{
+    JNIEnv* env = GetJNIEnv();
+    if (!env || !mLayerClient)
+        return;
+    AutoLocalJNIFrame jniFrame(env, 0);
+    mLayerClient->ContentDocumentChanged(&jniFrame);
+}
+
+bool
+AndroidBridge::IsContentDocumentDisplayed()
+{
+    JNIEnv* env = GetJNIEnv();
+    if (!env || !mLayerClient)
+        return false;
+    AutoLocalJNIFrame jniFrame(env, 0);
+    return mLayerClient->IsContentDocumentDisplayed(&jniFrame);
+}
+
 bool
 AndroidBridge::ProgressiveUpdateCallback(bool aHasPendingNewThebesContent, const gfx::Rect& aDisplayPort, float aDisplayResolution, bool aDrawingCritical, gfx::Rect& aViewport, float& aScaleX, float& aScaleY)
 {

widget/android/AndroidBridge.h

@@ -191,6 +191,8 @@ public:
     nsresult CaptureThumbnail(nsIDOMWindow *window, int32_t bufW, int32_t bufH, int32_t tabId, jobject buffer);
     void SendThumbnail(jobject buffer, int32_t tabId, bool success);
     void GetDisplayPort(bool aPageSizeUpdate, bool aIsBrowserContentDisplayed, int32_t tabId, nsIAndroidViewport* metrics, nsIAndroidDisplayport** displayPort);
+    void ContentDocumentChanged();
+    bool IsContentDocumentDisplayed();
 
     bool ProgressiveUpdateCallback(bool aHasPendingNewThebesContent, const gfx::Rect& aDisplayPort, float aDisplayResolution, bool aDrawingCritical, gfx::Rect& aViewport, float& aScaleX, float& aScaleY);
 

widget/android/AndroidJavaWrappers.cpp

@@ -97,6 +97,8 @@ jmethodID AndroidGeckoLayerClient::jCreateFrameMethod = 0;
 jmethodID AndroidGeckoLayerClient::jActivateProgramMethod = 0;
 jmethodID AndroidGeckoLayerClient::jDeactivateProgramMethod = 0;
 jmethodID AndroidGeckoLayerClient::jGetDisplayPort = 0;
+jmethodID AndroidGeckoLayerClient::jContentDocumentChanged = 0;
+jmethodID AndroidGeckoLayerClient::jIsContentDocumentDisplayed = 0;
 jmethodID AndroidGeckoLayerClient::jViewportCtor = 0;
 jfieldID AndroidGeckoLayerClient::jDisplayportPosition = 0;
 jfieldID AndroidGeckoLayerClient::jDisplayportResolution = 0;
@@ -359,6 +361,8 @@ AndroidGeckoLayerClient::InitGeckoLayerClientClass(JNIEnv *jEnv)
     jActivateProgramMethod = getMethod("activateProgram", "()V");
     jDeactivateProgramMethod = getMethod("deactivateProgram", "()V");
     jGetDisplayPort = getMethod("getDisplayPort", "(ZZILorg/mozilla/gecko/gfx/ImmutableViewportMetrics;)Lorg/mozilla/gecko/gfx/DisplayPortMetrics;");
+    jContentDocumentChanged = getMethod("contentDocumentChanged", "()V");
+    jIsContentDocumentDisplayed = getMethod("isContentDocumentDisplayed", "()Z");
 
     jViewportClass = GetClassGlobalRef(jEnv, "org/mozilla/gecko/gfx/ImmutableViewportMetrics");
     jViewportCtor = GetMethodID(jEnv, jViewportClass, "<init>", "(FFFFFFFFFFFFF)V");
@@ -1068,6 +1072,18 @@ AndroidGeckoLayerClient::GetDisplayPort(AutoLocalJNIFrame *jniFrame, bool aPageS
     (*displayPort)->AddRef();
 }
 
+void
+AndroidGeckoLayerClient::ContentDocumentChanged(AutoLocalJNIFrame *jniFrame)
+{
+    jniFrame->GetEnv()->CallVoidMethod(wrapped_obj, jContentDocumentChanged);
+}
+
+bool
+AndroidGeckoLayerClient::IsContentDocumentDisplayed(AutoLocalJNIFrame *jniFrame)
+{
+    return jniFrame->GetEnv()->CallBooleanMethod(wrapped_obj, jIsContentDocumentDisplayed);
+}
+
 bool
 AndroidGeckoLayerClient::CreateFrame(AutoLocalJNIFrame *jniFrame, AndroidLayerRendererFrame& aFrame)
 {

widget/android/AndroidJavaWrappers.h

@@ -281,6 +281,8 @@ public:
     bool ActivateProgram(AutoLocalJNIFrame *jniFrame);
     bool DeactivateProgram(AutoLocalJNIFrame *jniFrame);
     void GetDisplayPort(AutoLocalJNIFrame *jniFrame, bool aPageSizeUpdate, bool aIsBrowserContentDisplayed, int32_t tabId, nsIAndroidViewport* metrics, nsIAndroidDisplayport** displayPort);
+    void ContentDocumentChanged(AutoLocalJNIFrame *jniFrame);
+    bool IsContentDocumentDisplayed(AutoLocalJNIFrame *jniFrame);
 
 protected:
     static jclass jGeckoLayerClientClass;
@@ -292,6 +294,8 @@ protected:
     static jmethodID jActivateProgramMethod;
     static jmethodID jDeactivateProgramMethod;
     static jmethodID jGetDisplayPort;
+    static jmethodID jContentDocumentChanged;
+    static jmethodID jIsContentDocumentDisplayed;
     static jmethodID jProgressiveUpdateCallbackMethod;
 
 public:

widget/android/nsIAndroidBridge.idl

@@ -41,10 +41,12 @@ interface nsIAndroidDisplayport : nsISupports {
   attribute float resolution;
 };
 
-[scriptable, uuid(bbb8e0d7-5cca-4ad0-88be-538ce6d04f63)]
+[scriptable, uuid(5aa0cfa5-377c-4f5e-8dcf-59ebd9482d65)]
 interface nsIAndroidBridge : nsISupports
 {
   AString handleGeckoMessage(in AString message);
   attribute nsIAndroidBrowserApp browserApp;
   nsIAndroidDisplayport getDisplayPort(in boolean aPageSizeUpdate, in boolean isBrowserContentDisplayed, in int32_t tabId, in nsIAndroidViewport metrics);
+  void contentDocumentChanged();
+  boolean isContentDocumentDisplayed();
 };