Bug 1443658 - Use an integer for target in NavigationDelegate.onLoadUri() target r=esawin
☠☠ backed out by dd3beebf0ce3 ☠ ☠
authorJames Willcox <snorp@snorp.net>
Wed, 28 Feb 2018 16:14:30 -0500
changeset 462128 69b649d8c1d16e4a78ff3d99159a1b76cbf20e13
parent 462127 919f86518ed3612e14295c15fad60b708f913557
child 462129 bbb5759485fe49ea30d27d0d002f00fcdb3680ee
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersesawin
bugs1443658
milestone60.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 1443658 - Use an integer for target in NavigationDelegate.onLoadUri() target r=esawin MozReview-Commit-ID: AXv8eHQ9sgG
mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationListenerTest.kt
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/TestRunnerActivity.java
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/util/Callbacks.kt
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.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
@@ -606,19 +606,19 @@ public class CustomTabsActivity extends 
 
     @Override
     public void onCanGoForward(GeckoSession session, boolean canGoForward) {
         mCanGoForward = canGoForward;
         updateMenuItemForward();
     }
 
     @Override
-    public boolean onLoadUri(final GeckoSession session, final String urlStr,
-                             final TargetWindow where) {
-        if (where != TargetWindow.NEW) {
+    public boolean onLoadRequest(final GeckoSession session, final String urlStr,
+                                 final int target) {
+        if (target != GeckoSession.NavigationDelegate.TARGET_WINDOW_NEW) {
             return false;
         }
 
         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 true;
--- a/mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java
+++ b/mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java
@@ -375,25 +375,25 @@ public class WebAppActivity extends AppC
 
     @Override // GeckoSession.ContentDelegate
     public void onFullScreen(GeckoSession session, boolean fullScreen) {
         updateFullScreenContent(fullScreen);
     }
 
     @Override
     public boolean onLoadUri(final GeckoSession session, final String urlStr,
-                             final TargetWindow where) {
+                             final int target) {
         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 true;
         }
 
-        if (mManifest.isInScope(uri) && where != TargetWindow.NEW) {
+        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 false;
         }
 
         if ("javascript".equals(uri.getScheme())) {
             // These URIs will fail the scope check but should still be loaded in the PWA.
             return false;
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationListenerTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationListenerTest.kt
@@ -39,23 +39,23 @@ class NavigationDelegateTest {
 
     @Test fun load() {
         sessionRule.session.loadTestPath(HELLO_HTML_PATH)
         sessionRule.waitForPageStop()
 
         sessionRule.forCallbacksDuringWait(object : Callbacks.NavigationDelegate {
             @AssertCalled(count = 1, order = intArrayOf(1))
             override fun onLoadUri(session: GeckoSession, uri: String,
-                                   where: GeckoSession.NavigationDelegate.TargetWindow): Boolean {
+                                   where: Int): Boolean {
                 assertThat("Session should not be null", session, notNullValue())
                 assertThat("URI should not be null", uri, notNullValue())
                 assertThat("URI should match", uri, endsWith(HELLO_HTML_PATH))
                 assertThat("Where should not be null", where, notNullValue())
                 assertThat("Where should match", where,
-                           equalTo(GeckoSession.NavigationDelegate.TargetWindow.CURRENT))
+                           equalTo(GeckoSession.NavigationDelegate.TARGET_WINDOW_CURRENT))
                 return false
             }
 
             @AssertCalled(count = 1, order = intArrayOf(2))
             override fun onLocationChange(session: GeckoSession, url: String) {
                 assertThat("Session should not be null", session, notNullValue())
                 assertThat("URL should not be null", url, notNullValue())
                 assertThat("URL should match", url, endsWith(HELLO_HTML_PATH))
@@ -85,20 +85,20 @@ class NavigationDelegateTest {
         sessionRule.waitForPageStop()
 
         sessionRule.session.reload()
         sessionRule.waitForPageStop()
 
         sessionRule.forCallbacksDuringWait(object : Callbacks.NavigationDelegate {
             @AssertCalled(count = 1, order = intArrayOf(1))
             override fun onLoadUri(session: GeckoSession, uri: String,
-                                   where: GeckoSession.NavigationDelegate.TargetWindow): Boolean {
+                                   where: Int): Boolean {
                 assertThat("URI should match", uri, endsWith(HELLO_HTML_PATH))
                 assertThat("Where should match", where,
-                           equalTo(GeckoSession.NavigationDelegate.TargetWindow.CURRENT))
+                           equalTo(GeckoSession.NavigationDelegate.TARGET_WINDOW_CURRENT))
                 return false
             }
 
             @AssertCalled(count = 1, order = intArrayOf(2))
             override fun onLocationChange(session: GeckoSession, url: String) {
                 assertThat("URL should match", url, endsWith(HELLO_HTML_PATH))
             }
 
@@ -134,20 +134,20 @@ class NavigationDelegateTest {
         })
 
         sessionRule.session.goBack()
         sessionRule.waitForPageStop()
 
         sessionRule.forCallbacksDuringWait(object : Callbacks.NavigationDelegate {
             @AssertCalled(count = 1, order = intArrayOf(1))
             override fun onLoadUri(session: GeckoSession, uri: String,
-                                   where: GeckoSession.NavigationDelegate.TargetWindow): Boolean {
+                                   where: Int): Boolean {
                 assertThat("URI should match", uri, endsWith(HELLO_HTML_PATH))
                 assertThat("Where should match", where,
-                           equalTo(GeckoSession.NavigationDelegate.TargetWindow.CURRENT))
+                           equalTo(GeckoSession.NavigationDelegate.TARGET_WINDOW_CURRENT))
                 return false
             }
 
             @AssertCalled(count = 1, order = intArrayOf(2))
             override fun onLocationChange(session: GeckoSession, url: String) {
                 assertThat("URL should match", url, endsWith(HELLO_HTML_PATH))
             }
 
@@ -168,20 +168,20 @@ class NavigationDelegateTest {
         })
 
         sessionRule.session.goForward()
         sessionRule.waitForPageStop()
 
         sessionRule.forCallbacksDuringWait(object : Callbacks.NavigationDelegate {
             @AssertCalled(count = 1, order = intArrayOf(1))
             override fun onLoadUri(session: GeckoSession, uri: String,
-                                   where: GeckoSession.NavigationDelegate.TargetWindow): Boolean {
+                                   where: Int): Boolean {
                 assertThat("URI should match", uri, endsWith(HELLO2_HTML_PATH))
                 assertThat("Where should match", where,
-                           equalTo(GeckoSession.NavigationDelegate.TargetWindow.CURRENT))
+                           equalTo(GeckoSession.NavigationDelegate.TARGET_WINDOW_CURRENT))
                 return false
             }
 
             @AssertCalled(count = 1, order = intArrayOf(2))
             override fun onLocationChange(session: GeckoSession, url: String) {
                 assertThat("URL should match", url, endsWith(HELLO2_HTML_PATH))
             }
 
@@ -201,17 +201,17 @@ class NavigationDelegateTest {
             }
         })
     }
 
     @Test fun onLoadUri_returnTrueCancelsLoad() {
         sessionRule.delegateDuringNextWait(object : Callbacks.NavigationDelegate {
             @AssertCalled(count = 2)
             override fun onLoadUri(session: GeckoSession, uri: String,
-                                   where: GeckoSession.NavigationDelegate.TargetWindow): Boolean {
+                                   where: Int): Boolean {
                 return uri.endsWith(HELLO_HTML_PATH)
             }
         })
 
         sessionRule.session.loadTestPath(HELLO_HTML_PATH)
         sessionRule.session.loadTestPath(HELLO2_HTML_PATH)
         sessionRule.waitForPageStop()
 
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/TestRunnerActivity.java
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/TestRunnerActivity.java
@@ -35,17 +35,17 @@ public class TestRunnerActivity extends 
         }
 
         @Override
         public void onCanGoForward(GeckoSession session, boolean canGoForward) {
 
         }
 
         @Override
-        public boolean onLoadUri(GeckoSession session, String uri, TargetWindow where) {
+        public boolean onLoadUri(GeckoSession session, String uri, int target) {
             // Allow Gecko to load all URIs
             return false;
         }
 
         @Override
         public void onNewSession(GeckoSession session, String uri, GeckoSession.Response<GeckoSession> response) {
             response.respond(createSession(session.getSettings()));
         }
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/util/Callbacks.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/util/Callbacks.kt
@@ -35,17 +35,17 @@ class Callbacks private constructor() {
         }
 
         override fun onCanGoBack(session: GeckoSession, canGoBack: Boolean) {
         }
 
         override fun onCanGoForward(session: GeckoSession, canGoForward: Boolean) {
         }
 
-        override fun onLoadUri(session: GeckoSession, uri: String, where: GeckoSession.NavigationDelegate.TargetWindow): Boolean {
+        override fun onLoadUri(session: GeckoSession, uri: String, where: Int): Boolean {
             return false;
         }
 
         override fun onNewSession(session: GeckoSession, uri: String, response: GeckoSession.Response<GeckoSession>) {
             response.respond(null)
         }
     }
 
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
@@ -122,33 +122,42 @@ public class GeckoSession extends LayerS
         new GeckoSessionHandler<NavigationDelegate>(
             "GeckoViewNavigation", this,
             new String[]{
                 "GeckoView:LocationChange",
                 "GeckoView:OnLoadUri",
                 "GeckoView:OnNewSession"
             }
         ) {
+            // This needs to match nsIBrowserDOMWindow.idl
+            private int convertGeckoTarget(int geckoTarget) {
+                switch (geckoTarget) {
+                    case 0: // OPEN_DEFAULTWINDOW
+                    case 1: // OPEN_CURRENTWINDOW
+                        return NavigationDelegate.TARGET_WINDOW_CURRENT;
+                    default: // OPEN_NEWWINDOW, OPEN_NEWTAB, OPEN_SWITCHTAB
+                        return NavigationDelegate.TARGET_WINDOW_NEW;
+                }
+            }
+
             @Override
             public void handleMessage(final NavigationDelegate delegate,
                                       final String event,
                                       final GeckoBundle message,
                                       final EventCallback callback) {
                 if ("GeckoView:LocationChange".equals(event)) {
                     delegate.onLocationChange(GeckoSession.this,
                                               message.getString("uri"));
                     delegate.onCanGoBack(GeckoSession.this,
                                          message.getBoolean("canGoBack"));
                     delegate.onCanGoForward(GeckoSession.this,
                                             message.getBoolean("canGoForward"));
                 } else if ("GeckoView:OnLoadUri".equals(event)) {
                     final String uri = message.getString("uri");
-                    final NavigationDelegate.TargetWindow where =
-                        NavigationDelegate.TargetWindow.forGeckoValue(
-                            message.getInt("where"));
+                    final int where = convertGeckoTarget(message.getInt("where"));
                     final boolean result =
                         delegate.onLoadUri(GeckoSession.this, uri, where);
                     callback.sendSuccess(result);
                 } else if ("GeckoView:OnNewSession".equals(event)) {
                     final String uri = message.getString("uri");
                     delegate.onNewSession(GeckoSession.this, uri,
                         new Response<GeckoSession>() {
                             @Override
@@ -1394,59 +1403,31 @@ public class GeckoSession extends LayerS
 
         /**
         * The view's ability to go forward has changed.
         * @param session The GeckoSession that initiated the callback.
         * @param canGoForward The new value for the ability.
         */
         void onCanGoForward(GeckoSession session, boolean canGoForward);
 
-        enum TargetWindow {
-            DEFAULT(0),
-            CURRENT(1),
-            NEW(2);
-
-            private static final TargetWindow[] sValues = TargetWindow.values();
-            private int mValue;
-
-            private TargetWindow(int value) {
-                mValue = value;
-            }
-
-            public static TargetWindow forValue(int value) {
-                return sValues[value];
-            }
-
-            public static TargetWindow forGeckoValue(int value) {
-                // DEFAULT(0),
-                // CURRENT(1),
-                // NEW(2),
-                // NEWTAB(3),
-                // SWITCHTAB(4);
-                final TargetWindow[] sMap = {
-                    DEFAULT,
-                    CURRENT,
-                    NEW,
-                    NEW,
-                    NEW
-                };
-                return sMap[value];
-            }
-        }
+        public static final int TARGET_WINDOW_NONE = 0;
+        public static final int TARGET_WINDOW_CURRENT = 1;
+        public static final int TARGET_WINDOW_NEW = 2;
 
         /**
-        * A request to open an URI.
-        * @param session The GeckoSession that initiated the callback.
-        * @param uri The URI to be loaded.
-        * @param where The target window.
-        *
-        * @return Whether or not the load was handled. Returning false will allow Gecko
-        *         to continue the load as normal.
-        */
-        boolean onLoadUri(GeckoSession session, String uri, TargetWindow where);
+         * A request to open an URI.
+         * @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
+         *               TARGET_WINDOW_*.
+         *
+         * @return Whether or not the load was handled. Returning false will allow Gecko
+         *         to continue the load as normal.
+         */
+        boolean onLoadUri(GeckoSession session, String uri, int target);
 
         /**
         * 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.
--- 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
@@ -344,18 +344,18 @@ public class GeckoViewActivity extends A
         }
 
         @Override
         public void onCanGoForward(GeckoSession session, boolean value) {
         }
 
         @Override
         public boolean onLoadUri(final GeckoSession session, final String uri,
-                                 final TargetWindow where) {
-            Log.d(LOGTAG, "onLoadUri=" + uri + " where=" + where);
+                                 final int target) {
+            Log.d(LOGTAG, "onLoadUri=" + uri + " where=" + target);
             return false;
         }
 
         @Override
         public void onNewSession(final GeckoSession session, final String uri, Response<GeckoSession> response) {
             response.respond(null);
         }
     }