Bug 1476106 - Part 3 - Move GeckoScreenOrientation updates into GeckoView. r=snorp
authorJan Henning <jh+bugzilla@buttercookie.de>
Sun, 12 Aug 2018 13:31:59 +0200
changeset 431782 e936ef6b4b50fc09d0ffb988cb563f4a79d7b34c
parent 431781 70bfe613b3253974eb3a2d2562739a6fd6d4ed22
child 431783 67b327d75ce6ff41b854ce731739e0a6851674ca
push id106548
push userdvarga@mozilla.com
push dateWed, 15 Aug 2018 22:25:34 +0000
treeherdermozilla-inbound@baba90c7c28f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1476106
milestone63.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 1476106 - Part 3 - Move GeckoScreenOrientation updates into GeckoView. r=snorp By moving the calls to GeckoScreenOrientation.update() into GeckoView, any app using a GeckoView will automatically update the screen orientation in Gecko, too, without any further action being required by the embedding app. The synchronisation around GeckoScreenOrientation.update()/(un)lock() is required for the following scenario: If (un)locking of the screen orientation as requested by Gecko caused the actual screen orientation of the app to change, there are two ways in which this will cause our internal screen orientation to be updated: 1. Either the call to delegate.setRequestedOrientationForCurrentActivity (happening on the Gecko thread if the original request to (un)lock came from Gecko) returns first and update() is subsequently first called from the Gecko thread, too, meaning the onOrientationChange notification to Gecko can occur synchronously as well. In that case, as soon as (un)lock returns to Gecko, querying our internal screen orientation will return the correct value. 2. Or else the GeckoView.onConfigurationChanged() call resulting from the screen rotation manages to call GeckoScreenOrientation.update() first and does so from the Android UI thread. This means that the onOrientationChange notification will be redispatched asynchronously to the Gecko thread, while the Gecko thread's call to GeckoScreenOrientation.update() will return early without doing any work, as the screen orientation had already been previously updated by the UI thread. As a result,there will be a period of time between the Gecko thread returning from GeckoScreenOrientation.(un)lock() and the onOrientationChange notification finally executing where querying the internal screen orientation will not yet return the new orientation. This can cause problems for Gecko (test) code that expects to (un)lock the orientation and then be able to immediately query the new, changed orientation after the call to (un)lock returns. Without additional measures in place, there are no guarantees at what point the GeckoView will receive the onConfigurationChanged() call in relation to the request to change the activity's orientation making its way back to (un)lock(). Therefore, we add synchronisation such that no other thread will be able to up- date the screen orientation in GeckoScreenOrientation while another thread is still busy (un)locking the screen orientation. MozReview-Commit-ID: 5s5NEJcuS0p
mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoScreenOrientation.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoView.java
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ -1062,18 +1062,16 @@ public abstract class GeckoApp extends G
             }
         } else if (savedInstanceState != null) {
             // Bug 896992 - This intent has already been handled; reset the intent.
             setIntent(new Intent(Intent.ACTION_MAIN));
         }
 
         super.onCreate(savedInstanceState);
 
-        GeckoScreenOrientation.getInstance().update(getResources().getConfiguration().orientation);
-
         setContentView(getLayout());
 
         // Set up Gecko layout.
         mRootLayout = (RelativeLayout) findViewById(R.id.root_layout);
         mGeckoLayout = (RelativeLayout) findViewById(R.id.gecko_layout);
         mMainLayout = (RelativeLayout) findViewById(R.id.main_layout);
         mLayerView = (GeckoView) findViewById(R.id.layer_view);
 
@@ -2172,21 +2170,16 @@ public abstract class GeckoApp extends G
         Log.d(LOGTAG, "onConfigurationChanged: " + newConfig.locale);
 
         final LocaleManager localeManager = BrowserLocaleManager.getInstance();
         final Locale changed = localeManager.onSystemConfigurationChanged(this, getResources(), newConfig, mLastLocale);
         if (changed != null) {
             onLocaleChanged(Locales.getLanguageTag(changed));
         }
 
-        // onConfigurationChanged is not called for 180 degree orientation changes,
-        // we will miss such rotations and the screen orientation will not be
-        // updated.
-        GeckoScreenOrientation.getInstance().update(newConfig.orientation);
-
         if (mPromptService != null) {
             mPromptService.changePromptOrientation(newConfig.orientation);
         }
 
         super.onConfigurationChanged(newConfig);
     }
 
     public String getContentProcessName() {
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoScreenOrientation.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoScreenOrientation.java
@@ -149,17 +149,17 @@ public class GeckoScreenOrientation {
     /*
      * Update screen orientation given the screen orientation.
      *
      * @param aScreenOrientation
      *        Gecko screen orientation based on android orientation and rotation.
      *
      * @return Whether the screen orientation has changed.
      */
-    public boolean update(ScreenOrientation aScreenOrientation) {
+    public synchronized boolean update(ScreenOrientation aScreenOrientation) {
         if (mScreenOrientation == aScreenOrientation) {
             return false;
         }
         mScreenOrientation = aScreenOrientation;
         Log.d(LOGTAG, "updating to new orientation " + mScreenOrientation);
         notifyListeners(mScreenOrientation);
         if (mShouldNotify) {
             // Gecko expects a definite screen orientation, so we default to the
@@ -231,38 +231,42 @@ public class GeckoScreenOrientation {
      *        rotation.
      *
      * @return Whether the locking was successful.
      */
     public boolean lock(ScreenOrientation aScreenOrientation) {
         Log.d(LOGTAG, "locking to " + aScreenOrientation);
         final ScreenOrientationDelegate delegate = GeckoAppShell.getScreenOrientationDelegate();
         final int activityInfoOrientation = screenOrientationToActivityInfoOrientation(aScreenOrientation);
-        if (delegate.setRequestedOrientationForCurrentActivity(activityInfoOrientation)) {
-            update(aScreenOrientation);
-            return true;
-        } else {
-            return false;
+        synchronized (this) {
+            if (delegate.setRequestedOrientationForCurrentActivity(activityInfoOrientation)) {
+                update(aScreenOrientation);
+                return true;
+            } else {
+                return false;
+            }
         }
     }
 
     /**
      * Unlock and update screen orientation.
      *
      * @return Whether the unlocking was successful.
      */
     public boolean unlock() {
         Log.d(LOGTAG, "unlocking");
         final ScreenOrientationDelegate delegate = GeckoAppShell.getScreenOrientationDelegate();
         final int activityInfoOrientation = screenOrientationToActivityInfoOrientation(ScreenOrientation.DEFAULT);
-        if (delegate.setRequestedOrientationForCurrentActivity(activityInfoOrientation)) {
-            update();
-            return true;
-        } else {
-            return false;
+        synchronized (this) {
+            if (delegate.setRequestedOrientationForCurrentActivity(activityInfoOrientation)) {
+                update();
+                return true;
+            } else {
+                return false;
+            }
         }
     }
 
     /*
      * Combine the Android orientation and rotation to the Gecko orientation.
      *
      * @param aAndroidOrientation
      *        Android orientation from Configuration.orientation.
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java
@@ -10,16 +10,17 @@ import android.os.Parcel;
 import android.os.Parcelable;
 import android.content.Context;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.util.Log;
 
 import org.mozilla.gecko.EventDispatcher;
 import org.mozilla.gecko.GeckoAppShell;
+import org.mozilla.gecko.GeckoScreenOrientation;
 import org.mozilla.gecko.GeckoThread;
 import org.mozilla.gecko.PrefsHelper;
 import org.mozilla.gecko.util.BundleEventListener;
 import org.mozilla.gecko.util.EventCallback;
 import org.mozilla.gecko.util.GeckoBundle;
 import org.mozilla.gecko.util.ThreadUtils;
 
 import java.io.File;
@@ -251,16 +252,32 @@ public final class GeckoRuntime implemen
      * internal data.
      *
      * @return Profile directory
      */
     @NonNull public File getProfileDir() {
         return GeckoThread.getActiveProfile().getDir();
     }
 
+    /**
+     * Notify Gecko that the screen orientation has changed.
+     */
+    public void orientationChanged() {
+        GeckoScreenOrientation.getInstance().update();
+    }
+
+    /**
+     * Notify Gecko that the screen orientation has changed.
+     * @param newOrientation The new screen orientation, as retrieved e.g. from the current
+     *                       {@link android.content.res.Configuration}.
+     */
+    public void orientationChanged(int newOrientation) {
+        GeckoScreenOrientation.getInstance().update(newOrientation);
+    }
+
     @Override // Parcelable
     public int describeContents() {
         return 0;
     }
 
     @Override // Parcelable
     public void writeToParcel(Parcel out, int flags) {
         out.writeParcelable(mSettings, flags);
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoView.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoView.java
@@ -11,16 +11,17 @@ import org.mozilla.gecko.EventDispatcher
 import org.mozilla.gecko.gfx.DynamicToolbarAnimator;
 import org.mozilla.gecko.gfx.PanZoomController;
 import org.mozilla.gecko.gfx.GeckoDisplay;
 import org.mozilla.gecko.InputMethods;
 import org.mozilla.gecko.util.ActivityUtils;
 
 import android.app.Activity;
 import android.content.Context;
+import android.content.res.Configuration;
 import android.graphics.Canvas;
 import android.graphics.Color;
 import android.graphics.Rect;
 import android.graphics.Region;
 import android.os.Build;
 import android.os.Handler;
 import android.os.Parcel;
 import android.os.Parcelable;
@@ -336,16 +337,17 @@ public class GeckoView extends FrameLayo
     public void onAttachedToWindow() {
         if (mSession == null) {
             setSession(new GeckoSession(), GeckoRuntime.getDefault(getContext()));
         }
 
         if (!mSession.isOpen()) {
             mSession.open(mRuntime);
         }
+        mRuntime.orientationChanged();
 
         super.onAttachedToWindow();
     }
 
     @Override
     public void onDetachedFromWindow() {
         super.onDetachedFromWindow();
 
@@ -355,16 +357,28 @@ public class GeckoView extends FrameLayo
 
         // If we saved state earlier, we don't want to close the window.
         if (!mStateSaved && mSession.isOpen()) {
             mSession.close();
         }
     }
 
     @Override
+    protected void onConfigurationChanged(Configuration newConfig) {
+        super.onConfigurationChanged(newConfig);
+
+        if (mRuntime != null) {
+            // onConfigurationChanged is not called for 180 degree orientation changes,
+            // we will miss such rotations and the screen orientation will not be
+            // updated.
+            mRuntime.orientationChanged(newConfig.orientation);
+        }
+    }
+
+    @Override
     public boolean gatherTransparentRegion(final Region region) {
         // For detecting changes in SurfaceView layout, we take a shortcut here and
         // override gatherTransparentRegion, instead of registering a layout listener,
         // which is more expensive.
         if (mSurfaceView != null) {
             mDisplay.onGlobalLayout();
         }
         return super.gatherTransparentRegion(region);