Bug 1476106 - Part 3 - Move GeckoScreenOrientation updates into GeckoView. r?snorp draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Sun, 12 Aug 2018 13:31:59 +0200
changeset 828641 77fb5e22ad1d4abf8f1c2d46fa02326046c09bc0
parent 828049 99af621bca81b8798f03ef575eaf7c198523b6be
child 828642 6bfe49ecd0d200c4c6f103b49df8ad0e69c60f1a
push id118688
push usermozilla@buttercookie.de
push dateSun, 12 Aug 2018 17:47:36 +0000
reviewerssnorp
bugs1476106
milestone63.0a1
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;
@@ -249,16 +250,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);