Bug 1495786 - Replace ambiguous Booleans with AllowOrDeny, an enum with clearly named values. r=snorp,jchen
authorDylan Roeh <droeh@mozilla.com>
Tue, 16 Oct 2018 12:23:42 -0500
changeset 441517 af27cdf736f592782bb9f6d0a0bff62222a55e3e
parent 441516 000359cfa7019e7d2edc8fe3d442a5643a81ef61
child 441518 3939abb2b2c283dfdd09ea45b29d2d6fcb042dfa
push id34867
push usershindli@mozilla.com
push dateWed, 17 Oct 2018 00:55:53 +0000
treeherdermozilla-central@778427bb6353 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp, jchen
bugs1495786
milestone64.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 1495786 - Replace ambiguous Booleans with AllowOrDeny, an enum with clearly named values. r=snorp,jchen
mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/AllowOrDeny.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoResult.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/BasicGeckoViewPrompt.java
mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java
--- a/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
+++ b/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
@@ -51,16 +51,17 @@ import org.mozilla.gecko.text.TextSelect
 import org.mozilla.gecko.util.ActivityUtils;
 import org.mozilla.gecko.util.ColorUtil;
 import org.mozilla.gecko.util.GeckoBundle;
 import org.mozilla.gecko.util.IntentUtils;
 import org.mozilla.gecko.util.PackageUtil;
 import org.mozilla.gecko.webapps.WebApps;
 import org.mozilla.gecko.widget.ActionModePresenter;
 import org.mozilla.gecko.widget.GeckoPopupMenu;
+import org.mozilla.geckoview.AllowOrDeny;
 import org.mozilla.geckoview.GeckoResult;
 import org.mozilla.geckoview.GeckoSession;
 import org.mozilla.geckoview.GeckoSessionSettings;
 import org.mozilla.geckoview.GeckoView;
 
 import java.util.List;
 
 public class CustomTabsActivity extends AppCompatActivity
@@ -578,28 +579,28 @@ public class CustomTabsActivity extends 
 
     @Override
     public void onCanGoForward(GeckoSession session, boolean canGoForward) {
         mCanGoForward = canGoForward;
         updateMenuItemForward();
     }
 
     @Override
-    public GeckoResult<Boolean> onLoadRequest(final GeckoSession session, final String urlStr,
-                                              final int target,
-                                              final int flags) {
+    public GeckoResult<AllowOrDeny> onLoadRequest(final GeckoSession session, final String urlStr,
+                                                  final int target,
+                                                  final int flags) {
         if (target != GeckoSession.NavigationDelegate.TARGET_WINDOW_NEW) {
-            return GeckoResult.fromValue(false);
+            return GeckoResult.fromValue(AllowOrDeny.ALLOW);
         }
 
         final Uri uri = Uri.parse(urlStr);
         if (uri == null) {
             // We can't handle this, so deny it.
             Log.w(LOGTAG, "Failed to parse URL for navigation: " + urlStr);
-            return GeckoResult.fromValue(true);
+            return GeckoResult.fromValue(AllowOrDeny.DENY);
         }
 
         // Always use Fennec for these schemes.
         if ("http".equals(uri.getScheme()) || "https".equals(uri.getScheme()) ||
             "data".equals(uri.getScheme()) || "blob".equals(uri.getScheme())) {
             final Intent intent = new Intent(this, BrowserApp.class);
             intent.setAction(Intent.ACTION_VIEW);
             intent.setData(uri);
@@ -614,17 +615,17 @@ public class CustomTabsActivity extends 
             intent.setData(uri);
             try {
                 startActivity(intent);
             } catch (ActivityNotFoundException e) {
                 Log.w(LOGTAG, "No activity handler found for: " + urlStr);
             }
         }
 
-        return GeckoResult.fromValue(true);
+        return GeckoResult.fromValue(AllowOrDeny.DENY);
     }
 
     @Override
     public GeckoResult<GeckoSession> onNewSession(final GeckoSession session, final String uri) {
         // We should never get here because we abort loads that need a new session in onLoadRequest()
         throw new IllegalStateException("Unexpected new session");
     }
 
--- a/mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java
+++ b/mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java
@@ -33,16 +33,17 @@ import org.mozilla.gecko.Telemetry;
 import org.mozilla.gecko.TelemetryContract;
 import org.mozilla.gecko.customtabs.CustomTabsActivity;
 import org.mozilla.gecko.permissions.Permissions;
 import org.mozilla.gecko.prompts.PromptService;
 import org.mozilla.gecko.text.TextSelection;
 import org.mozilla.gecko.util.ActivityUtils;
 import org.mozilla.gecko.util.ColorUtil;
 import org.mozilla.gecko.widget.ActionModePresenter;
+import org.mozilla.geckoview.AllowOrDeny;
 import org.mozilla.geckoview.GeckoResult;
 import org.mozilla.geckoview.GeckoSession;
 import org.mozilla.geckoview.GeckoSessionSettings;
 import org.mozilla.geckoview.GeckoView;
 
 public class WebAppActivity extends AppCompatActivity
                             implements ActionModePresenter,
                                        GeckoSession.ContentDelegate,
@@ -372,30 +373,30 @@ public class WebAppActivity extends AppC
     }
 
     @Override // GeckoSession.ContentDelegate
     public void onFullScreen(GeckoSession session, boolean fullScreen) {
         updateFullScreenContent(fullScreen);
     }
 
     @Override
-    public GeckoResult<Boolean> onLoadRequest(final GeckoSession session, final String urlStr,
-                                              final int target,
-                                              final int flags) {
+    public GeckoResult<AllowOrDeny> onLoadRequest(final GeckoSession session, final String urlStr,
+                                                  final int target,
+                                                  final int flags) {
         final Uri uri = Uri.parse(urlStr);
         if (uri == null) {
             // We can't really handle this, so deny it?
             Log.w(LOGTAG, "Failed to parse URL for navigation: " + urlStr);
-            return GeckoResult.fromValue(true);
+            return GeckoResult.fromValue(AllowOrDeny.DENY);
         }
 
         if (mManifest.isInScope(uri) && target != TARGET_WINDOW_NEW) {
             // This is in scope and wants to load in the same frame, so
             // let Gecko handle it.
-            return GeckoResult.fromValue(false);
+            return GeckoResult.fromValue(AllowOrDeny.ALLOW);
         }
 
         if ("http".equals(uri.getScheme()) || "https".equals(uri.getScheme()) ||
             "data".equals(uri.getScheme()) || "blob".equals(uri.getScheme())) {
             final CustomTabsIntent.Builder builder = new CustomTabsIntent.Builder()
                 .addDefaultShareMenuItem()
                 .setStartAnimations(this, R.anim.slide_in_right, R.anim.slide_out_left)
                 .setExitAnimations(this, R.anim.slide_in_left, R.anim.slide_out_right);
@@ -414,17 +415,17 @@ public class WebAppActivity extends AppC
             intent.setData(uri);
             try {
                 startActivity(intent);
             } catch (ActivityNotFoundException e) {
                 Log.w(LOGTAG, "No activity handler found for: " + urlStr);
             }
         }
 
-        return GeckoResult.fromValue(true);
+        return GeckoResult.fromValue(AllowOrDeny.DENY);
     }
 
     @Override
     public GeckoResult<GeckoSession> onNewSession(final GeckoSession session, final String uri) {
         // We should never get here because we abort loads that need a new session in onLoadRequest()
         throw new IllegalStateException("Unexpected new session");
     }
 
new file mode 100644
--- /dev/null
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/AllowOrDeny.java
@@ -0,0 +1,14 @@
+/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
+ * vim: ts=4 sw=4 expandtab:
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.geckoview;
+
+/**
+ * This represents a decision to allow or deny a request.
+ */
+public enum AllowOrDeny {
+    ALLOW, DENY;
+}
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoResult.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoResult.java
@@ -144,16 +144,26 @@ public class GeckoResult<T> {
     private static final String LOGTAG = "GeckoResult";
 
     public static final class UncaughtException extends RuntimeException {
         public UncaughtException(final Throwable cause) {
             super(cause);
         }
     }
 
+    /**
+     * A GeckoResult that resolves to AllowOrDeny.ALLOW
+     */
+    public static final GeckoResult<AllowOrDeny> ALLOW = GeckoResult.fromValue(AllowOrDeny.ALLOW);
+
+    /**
+     * A GeckoResult that resolves to AllowOrDeny.DENY
+     */
+    public static final GeckoResult<AllowOrDeny> DENY = GeckoResult.fromValue(AllowOrDeny.DENY);
+
     private Handler mHandler;
     private boolean mComplete;
     private T mValue;
     private Throwable mError;
     private boolean mIsUncaughtError;
     private ArrayList<Runnable> mListeners;
 
     /**
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
@@ -313,29 +313,35 @@ public class GeckoSession extends LayerS
 
                         delegate.onLoadError(GeckoSession.this, uri,
                                              NavigationDelegate.ERROR_CATEGORY_URI,
                                              NavigationDelegate.ERROR_MALFORMED_URI);
 
                         return;
                     }
 
-                    final GeckoResult<Boolean> result =
+                    final GeckoResult<AllowOrDeny> result =
                         delegate.onLoadRequest(GeckoSession.this, uri, where, flags);
 
                     if (result == null) {
                         callback.sendSuccess(null);
                         return;
                     }
 
-                    result.then(new GeckoResult.OnValueListener<Boolean, Void>() {
+                    result.then(new GeckoResult.OnValueListener<AllowOrDeny, Void>() {
                         @Override
-                        public GeckoResult<Void> onValue(Boolean value) throws Throwable {
+                        public GeckoResult<Void> onValue(AllowOrDeny value) throws Throwable {
                             ThreadUtils.assertOnUiThread();
-                            callback.sendSuccess(value);
+                            if (value == AllowOrDeny.ALLOW) {
+                                callback.sendSuccess(false);
+                            } else  if (value == AllowOrDeny.DENY) {
+                                callback.sendSuccess(true);
+                            } else {
+                                callback.sendError("Invalid response");
+                            }
                             return null;
                         }
                     }, new GeckoResult.OnExceptionListener<Void>() {
                         @Override
                         public GeckoResult<Void> onException(Throwable exception) throws Throwable {
                             callback.sendError(exception.getMessage());
                             return null;
                         }
@@ -2036,28 +2042,34 @@ public class GeckoSession extends LayerS
                         }
                     }
                     mimeTypes = combined.toArray(new String[combined.size()]);
                 }
                 delegate.onFilePrompt(session, title, intMode, mimeTypes, cb);
                 break;
             }
             case "popup": {
-                GeckoResult<Boolean> res = delegate.onPopupRequest(session, message.getString("targetUri"));
+                GeckoResult<AllowOrDeny> res = delegate.onPopupRequest(session, message.getString("targetUri"));
 
                 if (res == null) {
                     // Keep the popup blocked if the delegate returns null
                     callback.sendSuccess(false);
                     return;
                 }
 
-                res.then(new GeckoResult.OnValueListener<Boolean, Void>() {
+                res.then(new GeckoResult.OnValueListener<AllowOrDeny, Void>() {
                     @Override
-                    public GeckoResult<Void> onValue(Boolean value) throws Throwable {
-                        callback.sendSuccess(value);
+                    public GeckoResult<Void> onValue(AllowOrDeny value) throws Throwable {
+                        if (value == AllowOrDeny.ALLOW) {
+                            callback.sendSuccess(true);
+                        } else if (value == AllowOrDeny.DENY) {
+                            callback.sendSuccess(false);
+                        } else {
+                            callback.sendError("Invalid response");
+                        }
                         return null;
                     }
                 }, new GeckoResult.OnExceptionListener<Void>() {
                     @Override
                     public GeckoResult<Void> onException(Throwable exception) throws Throwable {
                         callback.sendError("Failed to get popup-blocking decision");
                         return null;
                     }
@@ -2564,25 +2576,25 @@ public class GeckoSession extends LayerS
          * @param session The GeckoSession that initiated the callback.
          * @param uri The URI to be loaded.
          * @param target The target where the window has requested to open.
          *               One of {@link #TARGET_WINDOW_NONE TARGET_WINDOW_*}.
          * @param flags The load request flags.
          *              One or more of {@link #LOAD_REQUEST_IS_USER_TRIGGERED
          *              LOAD_REQUEST_*}.
          *
-         * @return A {@link GeckoResult} with a boolean value which indicates whether or
-         *         not the load was handled. If unhandled, Gecko will continue the
+         * @return A {@link GeckoResult} with a AllowOrDeny value which indicates whether
+         *         or not the load was handled. If unhandled, Gecko will continue the
          *         load as normal. If handled (true value), Gecko will abandon the load.
          *         A null return value is interpreted as false (unhandled).
          */
-        @Nullable GeckoResult<Boolean> onLoadRequest(@NonNull GeckoSession session,
-                                                     @NonNull String uri,
-                                                     @TargetWindow int target,
-                                                     @LoadRequestFlags int flags);
+        @Nullable GeckoResult<AllowOrDeny> onLoadRequest(@NonNull GeckoSession session,
+                                                         @NonNull String uri,
+                                                         @TargetWindow int target,
+                                                         @LoadRequestFlags int flags);
 
         /**
         * A request has been made to open a new session. The URI is provided only for
         * informational purposes. Do not call GeckoSession.loadUri() here. Additionally, the
         * returned GeckoSession must be a newly-created one.
         *
         * @param session The GeckoSession that initiated the callback.
         * @param uri The URI to be loaded.
@@ -3137,20 +3149,20 @@ public class GeckoSession extends LayerS
 
         /**
          * Display a popup request prompt; this occurs when content attempts to open
          * a new window in a way that doesn't appear to be the result of user input.
          *
          * @param session GeckoSession that triggered the prompt
          * @param targetUri The target URI for the popup
          *
-         * @return A {@link GeckoResult} resolving to a Boolean which indicates
+         * @return A {@link GeckoResult} resolving to a AllowOrDeny which indicates
          *         whether or not the popup should be allowed to open.
          */
-        GeckoResult<Boolean> onPopupRequest(GeckoSession session, String targetUri);
+        GeckoResult<AllowOrDeny> onPopupRequest(GeckoSession session, String targetUri);
     }
 
     /**
      * GeckoSession applications implement this interface to handle content scroll
      * events.
      **/
     public interface ScrollDelegate {
         /**
--- a/mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/BasicGeckoViewPrompt.java
+++ b/mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/BasicGeckoViewPrompt.java
@@ -43,16 +43,17 @@ import android.widget.TimePicker;
 
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.Date;
 import java.util.Locale;
 
+import org.mozilla.geckoview.AllowOrDeny;
 import org.mozilla.geckoview.GeckoResult;
 import org.mozilla.geckoview.GeckoSession;
 import org.mozilla.geckoview.GeckoSession.PermissionDelegate.MediaSource;
 
 final class BasicGeckoViewPrompt implements GeckoSession.PromptDelegate {
     protected static final String LOGTAG = "BasicGeckoViewPrompt";
 
     private final Activity mActivity;
@@ -911,12 +912,12 @@ final class BasicGeckoViewPrompt impleme
 
     public void onMediaPrompt(final GeckoSession session, final String title,
                                final MediaSource[] video, final MediaSource[] audio,
                                final GeckoSession.PermissionDelegate.MediaCallback callback) {
         onMediaPrompt(session, title, video, audio, null, null, callback);
     }
 
     @Override
-    public GeckoResult<Boolean> onPopupRequest(final GeckoSession session, final String targetUri) {
-        return GeckoResult.fromValue(true);
+    public GeckoResult<AllowOrDeny> onPopupRequest(final GeckoSession session, final String targetUri) {
+        return GeckoResult.fromValue(AllowOrDeny.ALLOW);
     }
 }
--- a/mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java
+++ b/mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java
@@ -1,15 +1,16 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.geckoview_example;
 
+import org.mozilla.geckoview.AllowOrDeny;
 import org.mozilla.geckoview.BasicSelectionActionDelegate;
 import org.mozilla.geckoview.GeckoResult;
 import org.mozilla.geckoview.GeckoRuntime;
 import org.mozilla.geckoview.GeckoRuntimeSettings;
 import org.mozilla.geckoview.GeckoSession;
 import org.mozilla.geckoview.GeckoSession.TrackingProtectionDelegate;
 import org.mozilla.geckoview.GeckoSessionSettings;
 import org.mozilla.geckoview.GeckoView;
@@ -626,21 +627,21 @@ public class GeckoViewActivity extends A
         }
 
         @Override
         public void onCanGoForward(GeckoSession session, boolean canGoForward) {
             mCanGoForward = canGoForward;
         }
 
         @Override
-        public GeckoResult<Boolean> onLoadRequest(final GeckoSession session, final String uri,
-                                                  final int target, final int flags) {
+        public GeckoResult<AllowOrDeny> onLoadRequest(final GeckoSession session, final String uri,
+                                                       final int target, final int flags) {
             Log.d(LOGTAG, "onLoadRequest=" + uri + " where=" + target +
                   " flags=" + flags);
-            return GeckoResult.fromValue(false);
+            return GeckoResult.fromValue(AllowOrDeny.ALLOW);
         }
 
         @Override
         public GeckoResult<GeckoSession> onNewSession(final GeckoSession session, final String uri) {
             GeckoSession newSession = new GeckoSession(session.getSettings());
 
             Intent intent = new Intent(GeckoViewActivity.this, SessionActivity.class);
             intent.setFlags(Intent.FLAG_ACTIVITY_CLEAR_WHEN_TASK_RESET);