Bug 1474454 - Use GeckoResult in GeckoSession.NavigationDelegate.onNewSession() r=jchen,droeh
authorJames Willcox <snorp@snorp.net>
Mon, 09 Jul 2018 17:22:57 -0500
changeset 426655 714b1d874ec9f70de9a1c5560ddf823cde96cee3
parent 426654 3ebb68824d935128e81ae6f71c966c8f90bfe124
child 426656 58c3113b9020991cd44aed34c629f15b22b5a9bd
push id34278
push useraciure@mozilla.com
push dateSun, 15 Jul 2018 09:53:15 +0000
treeherdermozilla-central@2a8f94a45fd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjchen, droeh
bugs1474454
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 1474454 - Use GeckoResult in GeckoSession.NavigationDelegate.onNewSession() r=jchen,droeh MozReview-Commit-ID: E59Scu8tnuq
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/ContentDelegateTest.kt
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/GeckoResultTest.java
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/TestRunnerActivity.java
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/rule/GeckoSessionTestRule.java
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/util/Callbacks.kt
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/GeckoViewActivity.java
--- a/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
+++ b/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
@@ -636,18 +636,17 @@ public class CustomTabsActivity extends 
                 Log.w(LOGTAG, "No activity handler found for: " + urlStr);
             }
         }
 
         return GeckoResult.fromValue(true);
     }
 
     @Override
-    public void onNewSession(final GeckoSession session, final String uri,
-                             final GeckoResponse<GeckoSession> response) {
+    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");
     }
 
     /* GeckoSession.ProgressDelegate */
     @Override
     public void onPageStart(GeckoSession session, String url) {
         mCurrentUrl = url;
--- a/mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java
+++ b/mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java
@@ -430,18 +430,17 @@ public class WebAppActivity extends AppC
                 Log.w(LOGTAG, "No activity handler found for: " + urlStr);
             }
         }
 
         return GeckoResult.fromValue(true);
     }
 
     @Override
-    public void onNewSession(final GeckoSession session, final String uri,
-                             final GeckoResponse<GeckoSession> response) {
+    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");
     }
 
     private void updateFullScreen() {
         boolean fullScreen = mIsFullScreenContent || mIsFullScreenMode;
         if (ActivityUtils.isFullScreen(this) == fullScreen) {
             return;
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ContentDelegateTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ContentDelegateTest.kt
@@ -43,17 +43,18 @@ class ContentDelegateTest : BaseSessionT
 
             @AssertCalled(count = 2)
             override fun onLoadRequest(session: GeckoSession, uri: String,
                                        where: Int, flags: Int): GeckoResult<Boolean>? {
                 return null
             }
 
             @AssertCalled(false)
-            override fun onNewSession(session: GeckoSession, uri: String, response: GeckoResponse<GeckoSession>) {
+            override fun onNewSession(session: GeckoSession, uri: String): GeckoResult<GeckoSession>? {
+                return null
             }
 
             @AssertCalled(count = 1)
             override fun onExternalResponse(session: GeckoSession, response: GeckoSession.WebResponseInfo) {
                 assertThat("Uri should start with data:", response.uri, startsWith("data:"))
                 assertThat("Content type should match", response.contentType, equalTo("text/plain"))
                 assertThat("Content length should be non-zero", response.contentLength, greaterThan(0L))
                 assertThat("Filename should match", response.filename, equalTo("download.txt"))
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/GeckoResultTest.java
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/GeckoResultTest.java
@@ -90,22 +90,28 @@ public class GeckoResultTest {
     @Test(expected = IllegalArgumentException.class)
     @UiThreadTest
     public void thenNoListeners() {
         GeckoResult.fromValue(42).then(null, null);
     }
 
     @Test
     @UiThreadTest
-    public void testEquals() {
-        final GeckoResult<Integer> result = GeckoResult.fromValue(42);
-        final GeckoResult<Integer> result2 = new GeckoResult<>(result);
+    public void testCopy() {
+        final GeckoResult<Integer> result = new GeckoResult<>(GeckoResult.fromValue(42));
+        result.then(new OnValueListener<Integer, Void>() {
+            @Override
+            public GeckoResult<Void> onValue(Integer value) throws Throwable {
+                assertThat("Value should match", value, equalTo(42));
+                done();
+                return null;
+            }
+        });
 
-        assertThat("Results should be equal", result, equalTo(result2));
-        assertThat("Hashcode should be equal", result.hashCode(), equalTo(result2.hashCode()));
+        waitUntilDone();
     }
 
     @Test(expected = IllegalStateException.class)
     @UiThreadTest
     public void completeMultiple() {
         final GeckoResult<Integer> deferred = new GeckoResult<Integer>();
         deferred.complete(42);
         deferred.complete(43);
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt
@@ -13,16 +13,17 @@ import org.mozilla.geckoview.test.rule.G
 import org.mozilla.geckoview.test.rule.GeckoSessionTestRule.ReuseSession
 import org.mozilla.geckoview.test.rule.GeckoSessionTestRule.Setting
 import org.mozilla.geckoview.test.rule.GeckoSessionTestRule.WithDevToolsAPI
 import org.mozilla.geckoview.test.util.Callbacks
 
 import android.support.test.filters.MediumTest
 import android.support.test.runner.AndroidJUnit4
 import org.hamcrest.Matchers.*
+import org.junit.Ignore
 import org.junit.Test
 import org.junit.runner.RunWith
 
 @RunWith(AndroidJUnit4::class)
 @MediumTest
 @ReuseSession(false)
 class NavigationDelegateTest : BaseSessionTest() {
 
@@ -126,18 +127,18 @@ class NavigationDelegateTest : BaseSessi
 
             @AssertCalled(count = 1, order = [2])
             override fun onCanGoForward(session: GeckoSession, canGoForward: Boolean) {
                 assertThat("Session should not be null", session, notNullValue())
                 assertThat("Cannot go forward", canGoForward, equalTo(false))
             }
 
             @AssertCalled(false)
-            override fun onNewSession(session: GeckoSession, uri: String,
-                                      response: GeckoResponse<GeckoSession>) {
+            override fun onNewSession(session: GeckoSession, uri: String): GeckoResult<GeckoSession>? {
+                return null
             }
         })
     }
 
     @Test fun load_dataUri() {
         val dataUrl = "data:,Hello%2C%20World!"
         sessionRule.session.loadUri(dataUrl);
         sessionRule.waitForPageStop();
@@ -312,18 +313,18 @@ class NavigationDelegateTest : BaseSessi
             }
 
             @AssertCalled(count = 1, order = [2])
             override fun onCanGoForward(session: GeckoSession, canGoForward: Boolean) {
                 assertThat("Cannot go forward", canGoForward, equalTo(false))
             }
 
             @AssertCalled(false)
-            override fun onNewSession(session: GeckoSession, uri: String,
-                                      response: GeckoResponse<GeckoSession>) {
+            override fun onNewSession(session: GeckoSession, uri: String): GeckoResult<GeckoSession>? {
+                return null
             }
         })
     }
 
     @Test fun goBackAndForward() {
         sessionRule.session.loadTestPath(HELLO_HTML_PATH)
         sessionRule.waitForPageStop()
 
@@ -361,18 +362,18 @@ class NavigationDelegateTest : BaseSessi
             }
 
             @AssertCalled(count = 1, order = [2])
             override fun onCanGoForward(session: GeckoSession, canGoForward: Boolean) {
                 assertThat("Can go forward", canGoForward, equalTo(true))
             }
 
             @AssertCalled(false)
-            override fun onNewSession(session: GeckoSession, uri: String,
-                                      response: GeckoResponse<GeckoSession>) {
+            override fun onNewSession(session: GeckoSession, uri: String): GeckoResult<GeckoSession>? {
+                return null
             }
         })
 
         sessionRule.session.goForward()
         sessionRule.waitForPageStop()
 
         sessionRule.forCallbacksDuringWait(object : Callbacks.NavigationDelegate {
             @AssertCalled(count = 1, order = [1])
@@ -395,18 +396,18 @@ class NavigationDelegateTest : BaseSessi
             }
 
             @AssertCalled(count = 1, order = [2])
             override fun onCanGoForward(session: GeckoSession, canGoForward: Boolean) {
                 assertThat("Cannot go forward", canGoForward, equalTo(false))
             }
 
             @AssertCalled(false)
-            override fun onNewSession(session: GeckoSession, uri: String,
-                                      response: GeckoResponse<GeckoSession>) {
+            override fun onNewSession(session: GeckoSession, uri: String): GeckoResult<GeckoSession>? {
+                return null
             }
         })
     }
 
     @Test fun onLoadUri_returnTrueCancelsLoad() {
         sessionRule.delegateDuringNextWait(object : Callbacks.NavigationDelegate {
             @AssertCalled(count = 2)
             override fun onLoadRequest(session: GeckoSession, uri: String,
@@ -448,18 +449,19 @@ class NavigationDelegateTest : BaseSessi
                                        where: Int, flags: Int): GeckoResult<Boolean>? {
                 assertThat("URI should be correct", uri, endsWith(NEW_SESSION_CHILD_HTML_PATH))
                 assertThat("Where should be correct", where,
                            equalTo(GeckoSession.NavigationDelegate.TARGET_WINDOW_NEW))
                 return null
             }
 
             @AssertCalled(count = 1, order = [2])
-            override fun onNewSession(session: GeckoSession, uri: String, response: GeckoResponse<GeckoSession>) {
+            override fun onNewSession(session: GeckoSession, uri: String): GeckoResult<GeckoSession>? {
                 assertThat("URI should be correct", uri, endsWith(NEW_SESSION_CHILD_HTML_PATH))
+                return null
             }
         })
     }
 
     @WithDevToolsAPI
     @Test fun onNewSession_calledForTargetBlankLink() {
         // Disable popup blocker.
         sessionRule.setPrefsUntilTestEnd(mapOf("dom.disable_open_during_load" to false))
@@ -477,29 +479,30 @@ class NavigationDelegateTest : BaseSessi
                                        where: Int, flags: Int): GeckoResult<Boolean>? {
                 assertThat("URI should be correct", uri, endsWith(NEW_SESSION_CHILD_HTML_PATH))
                 assertThat("Where should be correct", where,
                            equalTo(GeckoSession.NavigationDelegate.TARGET_WINDOW_NEW))
                 return null
             }
 
             @AssertCalled(count = 1, order = [2])
-            override fun onNewSession(session: GeckoSession, uri: String, response: GeckoResponse<GeckoSession>) {
+            override fun onNewSession(session: GeckoSession, uri: String): GeckoResult<GeckoSession>? {
                 assertThat("URI should be correct", uri, endsWith(NEW_SESSION_CHILD_HTML_PATH))
+                return null
             }
         })
     }
 
     private fun delegateNewSession(): GeckoSession {
         val newSession = sessionRule.createClosedSession()
 
         sessionRule.session.delegateDuringNextWait(object : Callbacks.NavigationDelegate {
             @AssertCalled(count = 1)
-            override fun onNewSession(session: GeckoSession, uri: String, response: GeckoResponse<GeckoSession>) {
-                response.respond(newSession)
+            override fun onNewSession(session: GeckoSession, uri: String): GeckoResult<GeckoSession> {
+                return GeckoResult.fromValue(newSession)
             }
         })
 
         return newSession
     }
 
     @WithDevToolsAPI
     @Test fun onNewSession_childShouldLoad() {
@@ -586,34 +589,35 @@ class NavigationDelegateTest : BaseSessi
             override fun onLoadRequest(session: GeckoSession, uri: String,
                                        where: Int, flags: Int): GeckoResult<Boolean>? {
                 assertThat("URI must match", uri,
                            endsWith(forEachCall(NEW_SESSION_CHILD_HTML_PATH, NEW_SESSION_HTML_PATH)))
                 return null
             }
 
             @AssertCalled(count = 0)
-            override fun onNewSession(session: GeckoSession, uri: String, response: GeckoResponse<GeckoSession>) {
+            override fun onNewSession(session: GeckoSession, uri: String): GeckoResult<GeckoSession>? {
+                return null
             }
         })
     }
 
     @WithDevToolsAPI
-    @Test(expected = IllegalArgumentException::class)
+    @Test(expected = GeckoResult.UncaughtException::class)
     fun onNewSession_doesNotAllowOpened() {
         // Disable popup blocker.
         sessionRule.setPrefsUntilTestEnd(mapOf("dom.disable_open_during_load" to false))
 
         sessionRule.session.loadTestPath(NEW_SESSION_HTML_PATH)
         sessionRule.session.waitForPageStop()
 
         sessionRule.session.delegateDuringNextWait(object : Callbacks.NavigationDelegate {
             @AssertCalled(count = 1)
-            override fun onNewSession(session: GeckoSession, uri: String, response: GeckoResponse<GeckoSession>) {
-                response.respond(sessionRule.createOpenSession())
+            override fun onNewSession(session: GeckoSession, uri: String): GeckoResult<GeckoSession> {
+                return GeckoResult.fromValue(sessionRule.createOpenSession())
             }
         })
 
         sessionRule.session.evaluateJS("$('#targetBlankLink').click()")
 
         sessionRule.session.waitUntilCalled(GeckoSession.NavigationDelegate::class,
                                             "onNewSession")
     }
--- 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
@@ -53,18 +53,18 @@ public class TestRunnerActivity extends 
         @Override
         public GeckoResult<Boolean> onLoadRequest(GeckoSession session, String uri, int target,
                                                   int flags) {
             // Allow Gecko to load all URIs
             return GeckoResult.fromValue(false);
         }
 
         @Override
-        public void onNewSession(GeckoSession session, String uri, GeckoResponse<GeckoSession> response) {
-            response.respond(createBackgroundSession(session.getSettings()));
+        public GeckoResult<GeckoSession> onNewSession(GeckoSession session, String uri) {
+            return GeckoResult.fromValue(createBackgroundSession(session.getSettings()));
         }
     };
 
     private GeckoSession.ContentDelegate mContentDelegate = new GeckoSession.ContentDelegate() {
         @Override
         public void onTitleChange(GeckoSession session, String title) {
 
         }
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/rule/GeckoSessionTestRule.java
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/rule/GeckoSessionTestRule.java
@@ -107,17 +107,17 @@ public class GeckoSessionTestRule extend
 
     static {
         try {
             sOnPageStart = GeckoSession.ProgressDelegate.class.getMethod(
                     "onPageStart", GeckoSession.class, String.class);
             sOnPageStop = GeckoSession.ProgressDelegate.class.getMethod(
                     "onPageStop", GeckoSession.class, boolean.class);
             sOnNewSession = GeckoSession.NavigationDelegate.class.getMethod(
-                    "onNewSession", GeckoSession.class, String.class, GeckoResponse.class);
+                    "onNewSession", GeckoSession.class, String.class);
             sOnCrash = GeckoSession.ContentDelegate.class.getMethod(
                     "onCrash", GeckoSession.class);
         } catch (final NoSuchMethodException e) {
             throw new RuntimeException(e);
         }
     }
 
     /**
@@ -1177,44 +1177,60 @@ public class GeckoSessionTestRule extend
                     }
 
                     if (isExternalDelegate) {
                         assertThat("External delegate should be registered",
                                    call, notNullValue());
                     }
                 }
 
-                if (call != null && sOnNewSession.equals(method)) {
-                    // We're delegating an onNewSession call.
-                    // Make sure we wait on the newly opened session, if any.
-                    final GeckoSession oldSession = (GeckoSession) args[0];
-                    @SuppressWarnings("unchecked")
-                    final GeckoResponse<GeckoSession> realResponse =
-                            (GeckoResponse<GeckoSession>) args[2];
-                    args[2] = new GeckoResponse<GeckoSession>() {
-                        @Override
-                        public void respond(final GeckoSession newSession) {
-                            realResponse.respond(newSession);
-                            // `realResponse` has opened the session at this point, so wait on it.
-                            if (oldSession.isOpen() && newSession != null) {
-                                GeckoSessionTestRule.this.waitForOpenSession(newSession);
-                            }
-                        }
-                    };
-                }
-
+                Object returnValue = null;
                 try {
                     mCurrentMethodCall = call;
-                    return method.invoke((call != null) ? call.target
+                    returnValue = method.invoke((call != null) ? call.target
                                                         : Callbacks.Default.INSTANCE, args);
                 } catch (final IllegalAccessException | InvocationTargetException e) {
                     throw unwrapRuntimeException(e);
                 } finally {
                     mCurrentMethodCall = null;
                 }
+
+                if (call == null || returnValue == null || !sOnNewSession.equals(method)) {
+                    return returnValue;
+                }
+
+                // We're delegating an onNewSession call.
+                // Make sure we wait on the newly opened session, if any.
+                final GeckoSession oldSession = (GeckoSession) args[0];
+
+                @SuppressWarnings("unchecked")
+                final GeckoResult<GeckoSession> result = (GeckoResult<GeckoSession>)returnValue;
+                final GeckoResult<GeckoSession> tmpResult = new GeckoResult<>();
+                result.then(new OnValueListener<GeckoSession, Void>() {
+                    @Override
+                    public GeckoResult<Void> onValue(final GeckoSession newSession) throws Throwable {
+                        tmpResult.complete(newSession);
+
+                        // GeckoSession has already hooked up its then() listener earlier,
+                        // so ours will run after. We can wait for the session to
+                        // open here.
+                        tmpResult.then(new OnValueListener<GeckoSession, Void>() {
+                            @Override
+                            public GeckoResult<Void> onValue(GeckoSession newSession) throws Throwable {
+                                if (oldSession.isOpen() && newSession != null) {
+                                    GeckoSessionTestRule.this.waitForOpenSession(newSession);
+                                }
+                                return null;
+                            }
+                        });
+                        return null;
+                    }
+                });
+
+                return tmpResult;
             }
         };
 
         final Class<?>[] classes = DEFAULT_DELEGATES.toArray(
                 new Class<?>[DEFAULT_DELEGATES.size()]);
         mCallbackProxy = Proxy.newProxyInstance(GeckoSession.class.getClassLoader(),
                                                 classes, recorder);
         mAllDelegates = new HashSet<>(DEFAULT_DELEGATES);
--- 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
@@ -53,18 +53,18 @@ class Callbacks private constructor() {
         override fun onCanGoForward(session: GeckoSession, canGoForward: Boolean) {
         }
 
         override fun onLoadRequest(session: GeckoSession, uri: String, where: Int,
                                    flags: Int): GeckoResult<Boolean>? {
             return null
         }
 
-        override fun onNewSession(session: GeckoSession, uri: String, response: GeckoResponse<GeckoSession>) {
-            response.respond(null)
+        override fun onNewSession(session: GeckoSession, uri: String): GeckoResult<GeckoSession>? {
+            return null;
         }
     }
 
     interface PermissionDelegate : GeckoSession.PermissionDelegate {
         override fun onAndroidPermissionsRequest(session: GeckoSession, permissions: Array<out String>, callback: GeckoSession.PermissionDelegate.Callback) {
             callback.reject()
         }
 
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoResult.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoResult.java
@@ -157,25 +157,23 @@ public class GeckoResult<T> {
     /**
      * Construct an incomplete GeckoResult. Call {@link #complete(Object)} or
      * {@link #completeExceptionally(Throwable)} in order to fulfill the result.
      */
     public GeckoResult() {
     }
 
     /**
-     * Construct a result from another result. Listeners are not copied.
+     * This constructs a result that is chained to the specified result.
      *
-      * @param from The {@link GeckoResult} to copy.
+     * @param from The {@link GeckoResult} to copy.
      */
     public GeckoResult(GeckoResult<T> from) {
         this();
-        mComplete = from.mComplete;
-        mValue = from.mValue;
-        mError = from.mError;
+        completeFrom(from);
     }
 
     /**
      * Construct a result that is completed with the specified value.
      *
      * @param value The value used to complete the newly created result.
      * @param <U> Type for the result.
      * @return The completed {@link GeckoResult}
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
@@ -220,37 +220,50 @@ public class GeckoSession extends LayerS
                         @Override
                         public GeckoResult<Void> onException(Throwable exception) throws Throwable {
                             callback.sendError(exception.getMessage());
                             return null;
                         }
                     });
                 } else if ("GeckoView:OnNewSession".equals(event)) {
                     final String uri = message.getString("uri");
-                    delegate.onNewSession(GeckoSession.this, uri,
-                        new GeckoResponse<GeckoSession>() {
-                            @Override
-                            public void respond(GeckoSession session) {
-                                if (session == null) {
-                                    callback.sendSuccess(null);
-                                    return;
-                                }
-
-                                if (session.isOpen()) {
-                                    throw new IllegalArgumentException("Must use an unopened GeckoSession instance");
-                                }
-
-                                if (GeckoSession.this.mWindow == null) {
-                                    callback.sendError("Session is not attached to a window");
-                                } else {
-                                    session.open(GeckoSession.this.mWindow.runtime);
-                                    callback.sendSuccess(session.getId());
-                                }
+                    final GeckoResult<GeckoSession> result = delegate.onNewSession(GeckoSession.this, uri);
+                    if (result == null) {
+                        callback.sendSuccess(null);
+                        return;
+                    }
+
+                    result.then(new GeckoResult.OnValueListener<GeckoSession, Void>() {
+                        @Override
+                        public GeckoResult<Void> onValue(GeckoSession session) throws Throwable {
+                            if (session == null) {
+                                callback.sendSuccess(null);
+                                return null;
+                            }
+
+                            if (session.isOpen()) {
+                                throw new IllegalArgumentException("Must use an unopened GeckoSession instance");
                             }
-                        });
+
+                            if (GeckoSession.this.mWindow == null) {
+                                callback.sendError("Session is not attached to a window");
+                            } else {
+                                session.open(GeckoSession.this.mWindow.runtime);
+                                callback.sendSuccess(session.getId());
+                            }
+
+                            return null;
+                        }
+                    }, new GeckoResult.OnExceptionListener<Void>() {
+                        @Override
+                        public GeckoResult<Void> onException(Throwable exception) throws Throwable {
+                            callback.sendError(exception.getMessage());
+                            return null;
+                        }
+                    });
                 }
             }
         };
 
     private final GeckoSessionHandler<ProgressDelegate> mProgressHandler =
         new GeckoSessionHandler<ProgressDelegate>(
             "GeckoViewProgress", this,
             new String[]{
@@ -2251,19 +2264,21 @@ public class GeckoSession extends LayerS
         /**
         * 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.
         *
-        * @param response A Response which will hold the returned GeckoSession
+        * @return A {@link GeckoResult} which holds the returned GeckoSession. May be null, in
+         *        which case the request for a new window by web content will fail. e.g.,
+         *        <code>window.open()</code> will return null.
         */
-        void onNewSession(GeckoSession session, String uri, GeckoResponse<GeckoSession> response);
+        @Nullable GeckoResult<GeckoSession> onNewSession(@NonNull GeckoSession session, @NonNull String uri);
     }
 
     /**
      * GeckoSession applications implement this interface to handle prompts triggered by
      * content in the GeckoSession, such as alerts, authentication dialogs, and select list
      * pickers.
      **/
     public interface PromptDelegate {
--- 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
@@ -565,27 +565,28 @@ public class GeckoViewActivity extends A
         public GeckoResult<Boolean> 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);
         }
 
         @Override
-        public void onNewSession(final GeckoSession session, final String uri, GeckoResponse<GeckoSession> response) {
+        public GeckoResult<GeckoSession> onNewSession(final GeckoSession session, final String uri) {
             GeckoSession newSession = new GeckoSession(session.getSettings());
-            response.respond(newSession);
 
             Intent intent = new Intent(GeckoViewActivity.this, SessionActivity.class);
             intent.setFlags(Intent.FLAG_ACTIVITY_CLEAR_WHEN_TASK_RESET);
             intent.setAction(Intent.ACTION_VIEW);
             intent.setData(Uri.parse(uri));
             intent.putExtra("session", newSession);
 
             startActivity(intent);
+
+            return GeckoResult.fromValue(newSession);
         }
     }
 
     private class ExampleTrackingProtectionDelegate implements GeckoSession.TrackingProtectionDelegate {
         private int mBlockedAds = 0;
         private int mBlockedAnalytics = 0;
         private int mBlockedSocial = 0;
         private int mBlockedContent = 0;