Bug 1480095 - Allow NavigationDelegate.onLoadError() to return URL for error page r=esawin,jchen draft
authorJames Willcox <snorp@snorp.net>
Fri, 03 Aug 2018 14:52:28 -0500
changeset 828056 ebf5551cd9e83aa72b99a61d6a07de9dc6cbf475
parent 828055 b0ec47e587d71b21697da7b762262ece6ad22020
child 828057 a368242c886ee306675071737440b8f831c83fcf
push id118624
push userbmo:snorp@snorp.net
push dateThu, 09 Aug 2018 20:07:39 +0000
reviewersesawin, jchen
bugs1480095
milestone63.0a1
Bug 1480095 - Allow NavigationDelegate.onLoadError() to return URL for error page r=esawin,jchen MozReview-Commit-ID: 4pgHD6oh4GY
mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java
mobile/android/chrome/geckoview/GeckoViewNavigationContent.js
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/BaseSessionTest.kt
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/util/Callbacks.kt
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
mobile/android/modules/geckoview/GeckoViewProgress.jsm
mobile/android/modules/geckoview/LoadURIDelegate.jsm
--- a/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
+++ b/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
@@ -649,18 +649,20 @@ public class CustomTabsActivity extends 
     }
 
     @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");
     }
 
-    public void onLoadError(final GeckoSession session, final String urlStr,
-                            final int category, final int error) {
+    @Override
+    public GeckoResult<String> onLoadError(final GeckoSession session, final String urlStr,
+                                           final int category, final int error) {
+        return null;
     }
 
     /* GeckoSession.ProgressDelegate */
     @Override
     public void onPageStart(GeckoSession session, String url) {
         mCurrentUrl = url;
         mCanStop = true;
         updateActionBar();
--- a/mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java
+++ b/mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java
@@ -450,18 +450,20 @@ public class WebAppActivity extends AppC
     }
 
     @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");
     }
 
-    public void onLoadError(final GeckoSession session, final String urlStr,
-                            final int category, final int error) {
+    @Override
+    public GeckoResult<String> onLoadError(final GeckoSession session, final String urlStr,
+                                           final int category, final int error) {
+        return null;
     }
 
     private void updateFullScreen() {
         boolean fullScreen = mIsFullScreenContent || mIsFullScreenMode;
         if (ActivityUtils.isFullScreen(this) == fullScreen) {
             return;
         }
 
--- a/mobile/android/chrome/geckoview/GeckoViewNavigationContent.js
+++ b/mobile/android/chrome/geckoview/GeckoViewNavigationContent.js
@@ -44,21 +44,15 @@ class GeckoViewNavigationContent extends
   }
 
   // nsILoadURIDelegate.
   handleLoadError(aUri, aError, aErrorModule) {
     debug `handleLoadError: uri=${aUri && aUri.spec}
                              uri2=${aUri && aUri.displaySpec}
                              error=${aError}`;
 
-    const handled = LoadURIDelegate.handleLoadError(content, this.eventDispatcher,
-                                                    aUri, aError, aErrorModule);
-    this.eventDispatcher.sendRequest({
-      type: "GeckoView:PageStop",
-      sucess: false,
-    });
-
-    return handled;
+    return LoadURIDelegate.handleLoadError(content, this.eventDispatcher,
+                                           aUri, aError, aErrorModule);
   }
 }
 
 let {debug, warn} = GeckoViewNavigationContent.initLogging("GeckoViewNavigation");
 let module = GeckoViewNavigationContent.create(this);
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/BaseSessionTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/BaseSessionTest.kt
@@ -26,17 +26,18 @@ import kotlin.reflect.KClass
 open class BaseSessionTest(noErrorCollector: Boolean = false) {
     companion object {
         const val CLICK_TO_RELOAD_HTML_PATH = "/assets/www/clickToReload.html"
         const val CONTENT_CRASH_URL = "about:crashcontent"
         const val DOWNLOAD_HTML_PATH = "/assets/www/download.html"
         const val HELLO_HTML_PATH = "/assets/www/hello.html"
         const val HELLO2_HTML_PATH = "/assets/www/hello2.html"
         const val INPUTS_PATH = "/assets/www/inputs.html"
-        const val INVALID_URI = "http://www.test.invalid/"
+        const val UNKNOWN_HOST_URI = "http://www.test.invalid/"
+        const val INVALID_URI = "not a valid uri"
         const val LOREM_IPSUM_HTML_PATH = "/assets/www/loremIpsum.html"
         const val NEW_SESSION_HTML_PATH = "/assets/www/newSession.html"
         const val NEW_SESSION_CHILD_HTML_PATH = "/assets/www/newSession_child.html"
         const val SAVE_STATE_PATH = "/assets/www/saveState.html"
         const val TITLE_CHANGE_HTML_PATH = "/assets/www/titleChange.html"
         const val TRACKERS_PATH = "/assets/www/trackers.html"
     }
 
--- 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
@@ -1,84 +1,248 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 4; indent-tabs-mode: nil; -*-
  * Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 package org.mozilla.geckoview.test
 
-import org.mozilla.gecko.util.GeckoBundle
 import org.mozilla.geckoview.GeckoResult
 import org.mozilla.geckoview.GeckoSession
 import org.mozilla.geckoview.GeckoSessionSettings
 import org.mozilla.geckoview.GeckoSession.TrackingProtectionDelegate;
 import org.mozilla.geckoview.test.rule.GeckoSessionTestRule.AssertCalled
 import org.mozilla.geckoview.test.rule.GeckoSessionTestRule.NullDelegate
 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() {
 
-    fun loadExpectError(testUri: String, expectedCategory: Int,
-                        expectedError: Int) {
+    fun loadExpectErrorWithErrorPage(testUri: String, expectedCategory: Int,
+                                     expectedError: Int,
+                                     errorPageUrl: String) {
+        sessionRule.delegateDuringNextWait(
+                object : Callbacks.ProgressDelegate, Callbacks.NavigationDelegate, Callbacks.ContentDelegate {
+                    @AssertCalled(count = 1, order = [1])
+                    override fun onLoadRequest(session: GeckoSession, uri: String,
+                                               where: Int, flags: Int): GeckoResult<Boolean>? {
+                        assertThat("URI should be " + testUri, uri, equalTo(testUri))
+                        return null
+                    }
+
+                    @AssertCalled(count = 1, order = [2])
+                    override fun onPageStart(session: GeckoSession, url: String) {
+                        assertThat("URI should be " + testUri, url, equalTo(testUri))
+                    }
+
+                    @AssertCalled(count = 1, order = [3])
+                    override fun onLoadError(session: GeckoSession, uri: String?,
+                                             category: Int, error: Int): GeckoResult<String>? {
+                        assertThat("Error category should match", category,
+                                equalTo(expectedCategory))
+                        assertThat("Error should match", error,
+                                equalTo(expectedError))
+                        return GeckoResult.fromValue(errorPageUrl)
+                    }
+
+                    @AssertCalled(count = 1, order = [4])
+                    override fun onPageStop(session: GeckoSession, success: Boolean) {
+                        assertThat("Load should fail", success, equalTo(false))
+                    }
+
+                    @AssertCalled(count = 1, order = [5])
+                    override fun onLocationChange(session: GeckoSession, url: String) {
+                        assertThat("URL should match", url, equalTo(testUri))
+                    }
+
+                    @AssertCalled(count = 1, order = [6])
+                    override fun onTitleChange(session: GeckoSession, title: String) {
+                        assertThat("Title should not be empty", title, not(isEmptyOrNullString()))
+                    }
+                })
+
+        sessionRule.session.loadUri(testUri);
+
+        // onTitleChange is called after onPageStop for an unknown reason
+        sessionRule.waitUntilCalled(object : Callbacks.ContentDelegate {
+            override fun onTitleChange(session: GeckoSession, title: String) {
+            }
+        });
+    }
+
+    fun loadExpectErrorNoErrorPage(testUri: String, expectedCategory: Int,
+                                   expectedError: Int) {
+        sessionRule.delegateDuringNextWait(
+                object : Callbacks.ProgressDelegate, Callbacks.NavigationDelegate, Callbacks.ContentDelegate {
+                    @AssertCalled(count = 1, order = [1])
+                    override fun onLoadRequest(session: GeckoSession, uri: String,
+                                               where: Int, flags: Int): GeckoResult<Boolean>? {
+                        assertThat("URI should be " + testUri, uri, equalTo(testUri))
+                        return null
+                    }
+
+                    @AssertCalled(count = 1, order = [2])
+                    override fun onPageStart(session: GeckoSession, url: String) {
+                        assertThat("URI should be " + testUri, url, equalTo(testUri))
+                    }
+
+                    @AssertCalled(count = 1, order = [3])
+                    override fun onLoadError(session: GeckoSession, uri: String?,
+                                             category: Int, error: Int): GeckoResult<String>? {
+                        assertThat("Error category should match", category,
+                                equalTo(expectedCategory))
+                        assertThat("Error should match", error,
+                                equalTo(expectedError))
+                        return null
+                    }
+
+                    @AssertCalled(count = 1, order = [4])
+                    override fun onPageStop(session: GeckoSession, success: Boolean) {
+                        assertThat("Load should fail", success, equalTo(false))
+                    }
+                })
+
         sessionRule.session.loadUri(testUri);
         sessionRule.waitForPageStop()
+    }
 
-        sessionRule.forCallbacksDuringWait(
-            object : Callbacks.ProgressDelegate, Callbacks.NavigationDelegate {
-            @AssertCalled(count = 1, order = [1])
-            override fun onLoadRequest(session: GeckoSession, uri: String,
-                                       where: Int, flags: Int): GeckoResult<Boolean>? {
-                assertThat("URI should be " + testUri, uri, equalTo(testUri))
+    fun loadExpectError(testUri: String, expectedCategory: Int,
+                        expectedError: Int) {
+        loadExpectErrorWithErrorPage(testUri, expectedCategory,
+                expectedError, createTestUrl(HELLO_HTML_PATH))
+        loadExpectErrorNoErrorPage(testUri, expectedCategory, expectedError)
+    }
+
+    fun loadExpectEarlyErrorWithErrorPage(testUri: String, expectedCategory: Int,
+                                          expectedError: Int,
+                                          errorPageUrl: String) {
+        sessionRule.delegateDuringNextWait(
+                object : Callbacks.ProgressDelegate, Callbacks.NavigationDelegate, Callbacks.ContentDelegate {
+
+                    @AssertCalled(false)
+                    override fun onPageStart(session: GeckoSession, url: String) {
+                        assertThat("URI should be " + testUri, url, equalTo(testUri))
+                    }
+
+                    @AssertCalled(count = 1, order = [1])
+                    override fun onLoadError(session: GeckoSession, uri: String?,
+                                             category: Int, error: Int): GeckoResult<String>? {
+                        assertThat("Error category should match", category,
+                                equalTo(expectedCategory))
+                        assertThat("Error should match", error,
+                                equalTo(expectedError))
+                        return GeckoResult.fromValue(errorPageUrl)
+                    }
+
+                    @AssertCalled(false)
+                    override fun onPageStop(session: GeckoSession, success: Boolean) {
+                    }
+
+                    @AssertCalled(count = 1, order = [2])
+                    override fun onTitleChange(session: GeckoSession, title: String) {
+                        assertThat("Title should not be empty", title, not(isEmptyOrNullString()));
+                    }
+                })
+
+        sessionRule.session.loadUri(testUri);
+
+        // onTitleChange is called after onPageStop for an unknown reason
+        sessionRule.waitUntilCalled(object : Callbacks.ContentDelegate {
+            override fun onTitleChange(session: GeckoSession, title: String) {
+            }
+        });
+    }
+
+    fun loadExpectEarlyErrorNoErrorPage(testUri: String, expectedCategory: Int,
+                                          expectedError: Int) {
+        sessionRule.session.loadUri(testUri);
+        sessionRule.waitUntilCalled(object : Callbacks.NavigationDelegate {
+            @AssertCalled
+            override fun onLoadError(session: GeckoSession, uri: String?, category: Int, error: Int): GeckoResult<String>? {
                 return null
             }
+        });
 
-            @AssertCalled(count = 1, order = [2])
-            override fun onLoadError(session: GeckoSession, uri: String,
-                                     category: Int, error: Int) {
-                assertThat("Error category should match", category,
-                           equalTo(expectedCategory))
-                assertThat("Error should match", error,
-                           equalTo(expectedError))
-            }
+        sessionRule.forCallbacksDuringWait(
+                object : Callbacks.ProgressDelegate, Callbacks.NavigationDelegate, Callbacks.ContentDelegate {
+
+                    @AssertCalled(false)
+                    override fun onPageStart(session: GeckoSession, url: String) {
+                        assertThat("URI should be " + testUri, url, equalTo(testUri))
+                    }
 
-            @AssertCalled(count = 1, order = [3])
-            override fun onPageStop(session: GeckoSession, success: Boolean) {
-                assertThat("Load should fail", success, equalTo(false))
-            }
-        })
+                    @AssertCalled(count = 1)
+                    override fun onLoadError(session: GeckoSession, uri: String?,
+                                             category: Int, error: Int): GeckoResult<String>? {
+                        assertThat("Error category should match", category,
+                                equalTo(expectedCategory))
+                        assertThat("Error should match", error,
+                                equalTo(expectedError))
+                        return null
+                    }
+
+                    @AssertCalled(false)
+                    override fun onPageStop(session: GeckoSession, success: Boolean) {
+                    }
+                })
+    }
+
+    fun loadExpectEarlyError(testUri: String, expectedCategory: Int,
+                             expectedError: Int) {
+        loadExpectEarlyErrorWithErrorPage(testUri, expectedCategory, expectedError, createTestUrl(HELLO_HTML_PATH))
+//        loadExpectEarlyErrorNoErrorPage(testUri, expectedCategory, expectedError)
     }
 
     @Test fun loadFileNotFound() {
         loadExpectError("file:///test.mozilla",
-                        GeckoSession.NavigationDelegate.ERROR_CATEGORY_URI,
-                        GeckoSession.NavigationDelegate.ERROR_FILE_NOT_FOUND)
+                GeckoSession.NavigationDelegate.ERROR_CATEGORY_URI,
+                GeckoSession.NavigationDelegate.ERROR_FILE_NOT_FOUND)
+    }
+
+    @Test fun loadFileNotFoundWith() {
+        loadExpectError("file:///test.mozilla",
+                GeckoSession.NavigationDelegate.ERROR_CATEGORY_URI,
+                GeckoSession.NavigationDelegate.ERROR_FILE_NOT_FOUND)
     }
 
     @Test fun loadUnknownHost() {
-        loadExpectError(INVALID_URI,
-                        GeckoSession.NavigationDelegate.ERROR_CATEGORY_URI,
-                        GeckoSession.NavigationDelegate.ERROR_UNKNOWN_HOST)
+        loadExpectError(UNKNOWN_HOST_URI,
+                GeckoSession.NavigationDelegate.ERROR_CATEGORY_URI,
+                GeckoSession.NavigationDelegate.ERROR_UNKNOWN_HOST)
+    }
+
+    @Test fun loadInvalidUri() {
+        loadExpectEarlyError(INVALID_URI,
+                GeckoSession.NavigationDelegate.ERROR_CATEGORY_URI,
+                GeckoSession.NavigationDelegate.ERROR_MALFORMED_URI)
     }
 
     @Test fun loadBadPort() {
-        loadExpectError("http://localhost:1/",
-                        GeckoSession.NavigationDelegate.ERROR_CATEGORY_NETWORK,
-                        GeckoSession.NavigationDelegate.ERROR_PORT_BLOCKED)
+        loadExpectEarlyError("http://localhost:1/",
+                GeckoSession.NavigationDelegate.ERROR_CATEGORY_NETWORK,
+                GeckoSession.NavigationDelegate.ERROR_PORT_BLOCKED)
+    }
+
+    @Test fun loadUntrusted() {
+        loadExpectError(if (sessionRule.env.isAutomation)
+            "https://expired.example.com/"
+        else
+            "https://expired.badssl.com/",
+                GeckoSession.NavigationDelegate.ERROR_CATEGORY_SECURITY,
+                GeckoSession.NavigationDelegate.ERROR_SECURITY_BAD_CERT);
     }
 
     @Setting(key = Setting.Key.USE_TRACKING_PROTECTION, value = "true")
     @Test fun trackingProtectionBasic() {
         val category = TrackingProtectionDelegate.CATEGORY_TEST;
         sessionRule.runtime.settings.trackingProtectionCategories = category
         sessionRule.session.loadTestPath(TRACKERS_PATH)
 
--- 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
@@ -58,18 +58,18 @@ public class TestRunnerActivity extends 
         }
 
         @Override
         public GeckoResult<GeckoSession> onNewSession(GeckoSession session, String uri) {
             return GeckoResult.fromValue(createBackgroundSession(session.getSettings()));
         }
 
         @Override
-        public void onLoadError(GeckoSession session, String uri, int category, int error) {
-
+        public GeckoResult<String> onLoadError(GeckoSession session, String uri, int category, int error) {
+            return null;
         }
     };
 
     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/util/Callbacks.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/util/Callbacks.kt
@@ -57,18 +57,19 @@ class Callbacks private constructor() {
                                    flags: Int): GeckoResult<Boolean>? {
             return null
         }
 
         override fun onNewSession(session: GeckoSession, uri: String): GeckoResult<GeckoSession>? {
             return null
         }
 
-        override fun onLoadError(session: GeckoSession, uri: String,
-                                 category: Int, error: Int) {
+        override fun onLoadError(session: GeckoSession, uri: String?,
+                                 category: Int, error: Int): GeckoResult<String>? {
+            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/GeckoSession.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
@@ -329,19 +329,35 @@ public class GeckoSession extends LayerS
                     final String uri = message.getString("uri");
                     final long errorCode = message.getLong("error");
                     final int errorModule = message.getInt("errorModule");
                     final int errorClass = message.getInt("errorClass");
                     final int error = convertGeckoError(errorCode, errorModule,
                                                         errorClass);
                     final int errorCat = getErrorCategory(errorModule, error);
 
-                    delegate.onLoadError(GeckoSession.this, uri, errorCat, error);
-
-                    callback.sendSuccess(!GeckoAppShell.isFennec());
+                    final GeckoResult<String> result = delegate.onLoadError(GeckoSession.this, uri, errorCat, error);
+                    if (result == null) {
+                        callback.sendSuccess(null);
+                        return;
+                    }
+
+                    result.then(new GeckoResult.OnValueListener<String, Void>() {
+                                    @Override
+                                    public GeckoResult<Void> onValue(@Nullable String url) throws Throwable {
+                                        callback.sendSuccess(url);
+                                        return null;
+                                    }
+                                }, new GeckoResult.OnExceptionListener<Void>() {
+                                    @Override
+                                    public GeckoResult<Void> onException(@NonNull Throwable exception) throws Throwable {
+                                        callback.sendError(exception.getMessage());
+                                        return null;
+                                    }
+                                });
                 } else if ("GeckoView:OnNewSession".equals(event)) {
                     final String uri = message.getString("uri");
                     final GeckoResult<GeckoSession> result = delegate.onNewSession(GeckoSession.this, uri);
                     if (result == null) {
                         callback.sendSuccess(null);
                         return;
                     }
 
@@ -2439,20 +2455,22 @@ public class GeckoSession extends LayerS
         public static final int ERROR_SAFEBROWSING_HARMFUL_URI = 0x47;
         public static final int ERROR_SAFEBROWSING_PHISHING_URI = 0x57;
 
         /**
          * @param session The GeckoSession that initiated the callback.
          * @param uri The URI that failed to load.
          * @param category The error category.
          * @param error The error type.
+         * @return A URL to an error page to display. Returning null will halt the load and
+         *         whatever is currently displayed will remain.
          */
-        void onLoadError(GeckoSession session, String uri,
-                         @LoadErrorCategory int category,
-                         @LoadError int error);
+        GeckoResult<String> onLoadError(GeckoSession session, String uri,
+                                        @LoadErrorCategory int category,
+                                        @LoadError int error);
     }
 
     /**
      * 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/modules/geckoview/GeckoViewProgress.jsm
+++ b/mobile/android/modules/geckoview/GeckoViewProgress.jsm
@@ -212,39 +212,47 @@ class GeckoViewProgress extends GeckoVie
     debug `onSettingsUpdate: ${settings}`;
 
     IdentityHandler.setUseTrackingProtection(!!settings.useTrackingProtection);
     IdentityHandler.setUsePrivateMode(!!settings.usePrivateMode);
   }
 
   onStateChange(aWebProgress, aRequest, aStateFlags, aStatus) {
     debug `onStateChange: isTopLevel=${aWebProgress.isTopLevel},
-                          flags=${aStateFlags}, status=${aStatus}`;
+                          flags=${aStateFlags}, status=${aStatus}
+                          loadType=${aWebProgress.loadType}`;
+
 
     if (!aWebProgress.isTopLevel) {
       return;
     }
 
     const uriSpec = aRequest.QueryInterface(Ci.nsIChannel).URI.displaySpec;
-    debug `onStateChange: uri=${uriSpec}`;
+    const isSuccess = aStatus == Cr.NS_OK;
+    const isStart = (aStateFlags & Ci.nsIWebProgressListener.STATE_START) != 0;
+    const isStop = (aStateFlags & Ci.nsIWebProgressListener.STATE_STOP) != 0;
 
-    if (aStateFlags & Ci.nsIWebProgressListener.STATE_START) {
+    debug `onStateChange: uri=${uriSpec} isSuccess=${isSuccess}
+           isStart=${isStart} isStop=${isStop}`;
+
+    if ((aStateFlags & Ci.nsIWebProgressListener.STATE_START)) {
       this._inProgress = true;
       const message = {
         type: "GeckoView:PageStart",
         uri: uriSpec,
       };
 
       this.eventDispatcher.sendRequest(message);
     } else if ((aStateFlags & Ci.nsIWebProgressListener.STATE_STOP) &&
                !aWebProgress.isLoadingDocument) {
       this._inProgress = false;
+
       let message = {
         type: "GeckoView:PageStop",
-        success: !aStatus
+        success: isSuccess
       };
 
       this.eventDispatcher.sendRequest(message);
     }
   }
 
   onSecurityChange(aWebProgress, aRequest, aState) {
     debug `onSecurityChange`;
@@ -267,23 +275,16 @@ class GeckoViewProgress extends GeckoVie
     this.eventDispatcher.sendRequest(message);
   }
 
   onLocationChange(aWebProgress, aRequest, aLocationURI, aFlags) {
     debug `onLocationChange: location=${aLocationURI.displaySpec},
                              flags=${aFlags}`;
 
     this._hostChanged = true;
-    if (aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_ERROR_PAGE) {
-      // We apparently don't get a STATE_STOP in onStateChange(), so emit PageStop here
-      this.eventDispatcher.sendRequest({
-        type: "GeckoView:PageStop",
-        success: false
-      });
-    }
   }
 
   // nsIObserver event handler
   observe(aSubject, aTopic, aData) {
     debug `observe: topic=${aTopic}`;
 
     switch (aTopic) {
       case "oop-frameloader-crashed": {
--- a/mobile/android/modules/geckoview/LoadURIDelegate.jsm
+++ b/mobile/android/modules/geckoview/LoadURIDelegate.jsm
@@ -47,28 +47,32 @@ const LoadURIDelegate = {
     try {
       let nssErrorsService = Cc["@mozilla.org/nss_errors_service;1"]
                              .getService(Ci.nsINSSErrorsService);
       errorClass = nssErrorsService.getErrorClass(aError);
     } catch (e) {}
 
     const msg = {
       type: "GeckoView:OnLoadError",
-      uri: aUri.spec,
+      uri: aUri ? aUri.spec : null,
       error: aError,
       errorModule: aErrorModule,
       errorClass
     };
 
-    let handled = undefined;
+    let errorPageURI = undefined;
     aEventDispatcher.sendRequestForResult(msg).then(response => {
-      handled = response;
+      try {
+        errorPageURI = Services.io.newURI(response);
+      } catch (e) {
+        errorPageURI = null;
+      }
     }, () => {
       // There was an error or listener was not registered in GeckoSession,
-      // treat as unhandled.
-      handled = false;
+      // return a null URI.
+      errorPageURI = null;
     });
     Services.tm.spinEventLoopUntil(() =>
-        aWindow.closed || handled !== undefined);
+        aWindow.closed || errorPageURI !== undefined);
 
-    return handled || false;
+    return errorPageURI;
   }
 };