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 489808 af27cdf736f592782bb9f6d0a0bff62222a55e3e
parent 489807 000359cfa7019e7d2edc8fe3d442a5643a81ef61
child 489809 3939abb2b2c283dfdd09ea45b29d2d6fcb042dfa
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewerssnorp, jchen
bugs1495786
milestone64.0a1
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);