Bug 1480095 - Allow NavigationDelegate.onLoadError() to return URL for error page r=esawin,jchen
authorJames Willcox <snorp@snorp.net>
Fri, 03 Aug 2018 14:52:28 -0500
changeset 432833 d49371d4563a373e0c701f385c59699f2f19dede
parent 432832 6cd69a56e853b2f2f3ef42e1ef406030cfbc9c1f
child 432834 6bb8792d978ca0b0b84a0fd1218260392b897ed1
push id34488
push usernerli@mozilla.com
push dateWed, 22 Aug 2018 16:28:54 +0000
treeherdermozilla-central@d6e4d3e69d4c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersesawin, jchen
bugs1480095
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 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/chrome/geckoview/geckoview.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/ProgressDelegateTest.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
@@ -623,18 +623,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
@@ -419,18 +419,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
@@ -9,56 +9,47 @@ ChromeUtils.import("resource://gre/modul
 XPCOMUtils.defineLazyModuleGetters(this, {
   ErrorPageEventHandler: "chrome://geckoview/content/ErrorPageEventHandler.js",
   LoadURIDelegate: "resource://gre/modules/LoadURIDelegate.jsm",
 });
 
 // Implements nsILoadURIDelegate.
 class GeckoViewNavigationContent extends GeckoViewContentModule {
   onInit() {
-    this.onEnable();
-  }
-
-  onEnable() {
-    debug `onEnable`;
-
     docShell.loadURIDelegate = this;
   }
 
-  onDisable() {
-    debug `onDisable`;
-
-    docShell.loadURIDelegate = null;
-  }
-
   // nsILoadURIDelegate.
   loadURI(aUri, aWhere, aFlags, aTriggeringPrincipal) {
     debug `loadURI: uri=${aUri && aUri.spec}
                     where=${aWhere} flags=${aFlags}`;
 
+    if (!this.enabled) {
+      return Promise.resolve(false);
+    }
+
     // TODO: Remove this when we have a sensible error API.
     if (aUri && aUri.displaySpec.startsWith("about:certerror")) {
       addEventListener("click", ErrorPageEventHandler, true);
     }
 
     return LoadURIDelegate.load(content, this.eventDispatcher,
                                 aUri, aWhere, aFlags);
   }
 
   // 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,
-    });
+    if (!this.enabled) {
+      Components.returnCode = Cr.NS_ERROR_ABORT;
+      return null;
+    }
 
-    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/chrome/geckoview/geckoview.js
+++ b/mobile/android/chrome/geckoview/geckoview.js
@@ -321,18 +321,16 @@ function startup() {
     onInit: {
       resource: "resource://gre/modules/GeckoViewContent.jsm",
       frameScript: "chrome://geckoview/content/GeckoViewContent.js",
     },
   }, {
     name: "GeckoViewNavigation",
     onInit: {
       resource: "resource://gre/modules/GeckoViewNavigation.jsm",
-    },
-    onEnable: {
       frameScript: "chrome://geckoview/content/GeckoViewNavigationContent.js",
     },
   }, {
     name: "GeckoViewProgress",
     onEnable: {
       resource: "resource://gre/modules/GeckoViewProgress.jsm",
       frameScript: "chrome://geckoview/content/GeckoViewProgressContent.js",
     },
--- 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
@@ -27,17 +27,18 @@ open class BaseSessionTest(noErrorCollec
     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 FORMS_HTML_PATH = "/assets/www/forms.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
@@ -15,71 +15,161 @@ import org.mozilla.geckoview.test.rule.G
 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.Assume.assumeThat
-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 testLoadErrorWithErrorPage(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))
+                    }
+                })
+
         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))
-                return null
-            }
+        if (errorPageUrl != null) {
+            sessionRule.waitUntilCalled(object : Callbacks.ContentDelegate, Callbacks.NavigationDelegate {
+                @AssertCalled(count = 1, order = [1])
+                override fun onLocationChange(session: GeckoSession, url: String) {
+                    assertThat("URL should match", url, equalTo(testUri))
+                }
+
+                @AssertCalled(count = 1, order = [2])
+                override fun onTitleChange(session: GeckoSession, title: String) {
+                    assertThat("Title should not be empty", title, not(isEmptyOrNullString()))
+                }
+            })
+        }
+    }
+
+    fun testLoadExpectError(testUri: String, expectedCategory: Int,
+                            expectedError: Int) {
+        testLoadErrorWithErrorPage(testUri, expectedCategory,
+                expectedError, createTestUrl(HELLO_HTML_PATH))
+        testLoadErrorWithErrorPage(testUri, expectedCategory,
+                expectedError, null)
+    }
+
+    fun testLoadEarlyErrorWithErrorPage(testUri: String, expectedCategory: Int,
+                                        expectedError: Int,
+                                        errorPageUrl: String?) {
+        sessionRule.delegateDuringNextWait(
+                object : Callbacks.ProgressDelegate, Callbacks.NavigationDelegate, Callbacks.ContentDelegate {
 
-            @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))
-            }
+                    @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(count = 1, order = [3])
-            override fun onPageStop(session: GeckoSession, success: Boolean) {
-                assertThat("Load should fail", success, equalTo(false))
-            }
-        })
+                    @AssertCalled(false)
+                    override fun onPageStop(session: GeckoSession, success: Boolean) {
+                    }
+                })
+
+        sessionRule.session.loadUri(testUri)
+        sessionRule.waitUntilCalled(Callbacks.NavigationDelegate::class, "onLoadError")
+
+        if (errorPageUrl != null) {
+            sessionRule.waitUntilCalled(object: Callbacks.ContentDelegate {
+                @AssertCalled(count = 1)
+                override fun onTitleChange(session: GeckoSession, title: String) {
+                    assertThat("Title should not be empty", title, not(isEmptyOrNullString()));
+                }
+            })
+        }
+    }
+
+    fun testLoadEarlyError(testUri: String, expectedCategory: Int,
+                           expectedError: Int) {
+        testLoadEarlyErrorWithErrorPage(testUri, expectedCategory, expectedError, createTestUrl(HELLO_HTML_PATH))
+        testLoadEarlyErrorWithErrorPage(testUri, expectedCategory, expectedError, null)
     }
 
     @Test fun loadFileNotFound() {
-        loadExpectError("file:///test.mozilla",
-                        GeckoSession.NavigationDelegate.ERROR_CATEGORY_URI,
-                        GeckoSession.NavigationDelegate.ERROR_FILE_NOT_FOUND)
+        testLoadExpectError("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)
+        testLoadExpectError(UNKNOWN_HOST_URI,
+                GeckoSession.NavigationDelegate.ERROR_CATEGORY_URI,
+                GeckoSession.NavigationDelegate.ERROR_UNKNOWN_HOST)
+    }
+
+    @Test fun loadInvalidUri() {
+        testLoadEarlyError(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)
+        testLoadEarlyError("http://localhost:1/",
+                GeckoSession.NavigationDelegate.ERROR_CATEGORY_NETWORK,
+                GeckoSession.NavigationDelegate.ERROR_PORT_BLOCKED)
+    }
+
+    @Test fun loadUntrusted() {
+        val uri = if (sessionRule.env.isAutomation) {
+            "https://expired.example.com/"
+        } else {
+            "https://expired.badssl.com/"
+        }
+        testLoadExpectError(uri,
+                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/ProgressDelegateTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ProgressDelegateTest.kt
@@ -75,25 +75,25 @@ class ProgressDelegateTest : BaseSession
             override fun onPageStop(session: GeckoSession, success: Boolean) {
                 assertThat("Session should not be null", session, notNullValue())
                 assertThat("Load should succeed", success, equalTo(true))
             }
         })
     }
 
     @Test fun multipleLoads() {
-        sessionRule.session.loadUri(INVALID_URI)
+        sessionRule.session.loadUri(UNKNOWN_HOST_URI)
         sessionRule.session.loadTestPath(HELLO_HTML_PATH)
         sessionRule.waitForPageStops(2)
 
         sessionRule.forCallbacksDuringWait(object : Callbacks.ProgressDelegate {
             @AssertCalled(count = 2, order = [1, 3])
             override fun onPageStart(session: GeckoSession, url: String) {
                 assertThat("URL should match", url,
-                           endsWith(forEachCall(INVALID_URI, HELLO_HTML_PATH)))
+                           endsWith(forEachCall(UNKNOWN_HOST_URI, HELLO_HTML_PATH)))
             }
 
             @AssertCalled(count = 2, order = [2, 4])
             override fun onPageStop(session: GeckoSession, success: Boolean) {
                 // The first load is certain to fail because of interruption by the second load
                 // or by invalid domain name, whereas the second load is certain to succeed.
                 assertThat("Success flag should match", success,
                            equalTo(forEachCall(false, true)))
--- 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
@@ -340,19 +340,47 @@ 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) {
+                        if (GeckoAppShell.isFennec()) {
+                            callback.sendSuccess(null);
+                        } else {
+                            callback.sendError("abort");
+                        }
+                        return;
+                    }
+
+                    result.then(new GeckoResult.OnValueListener<String, Void>() {
+                                    @Override
+                                    public GeckoResult<Void> onValue(@Nullable String url) throws Throwable {
+                                        if (url == null) {
+                                            if (GeckoAppShell.isFennec()) {
+                                                callback.sendSuccess(null);
+                                            } else {
+                                                callback.sendError("abort");
+                                            }
+                                        } else {
+                                            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;
                     }
 
@@ -2481,20 +2509,21 @@ 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 URI to display as an error. Returning null will halt the load entirely.
          */
-        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,46 @@ 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 (isStart) {
       this._inProgress = true;
       const message = {
         type: "GeckoView:PageStart",
         uri: uriSpec,
       };
 
       this.eventDispatcher.sendRequest(message);
-    } else if ((aStateFlags & Ci.nsIWebProgressListener.STATE_STOP) &&
-               !aWebProgress.isLoadingDocument) {
+    } else if (isStop && !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 +274,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
@@ -2,21 +2,24 @@
 /* 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/. */
 "use strict";
 
 var EXPORTED_SYMBOLS = ["LoadURIDelegate"];
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
+ChromeUtils.import("resource://gre/modules/GeckoViewUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetters(this, {
   Services: "resource://gre/modules/Services.jsm",
 });
 
+GeckoViewUtils.initLogging("LoadURIDelegate", this);
+
 const LoadURIDelegate = {
   // Delegate URI loading to the app.
   // Return whether the loading has been handled.
   load: function(aWindow, aEventDispatcher, aUri, aWhere, aFlags) {
     if (!aWindow) {
       return Promise.resolve(false);
     }
 
@@ -36,28 +39,33 @@ 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,
       error: aError,
       errorModule: aErrorModule,
       errorClass
     };
 
-    let handled = undefined;
+    let errorPageURI = undefined;
     aEventDispatcher.sendRequestForResult(msg).then(response => {
-      handled = response;
-    }, () => {
-      // There was an error or listener was not registered in GeckoSession,
-      // treat as unhandled.
-      handled = false;
+      try {
+        errorPageURI = Services.io.newURI(response);
+      } catch (e) {
+        warn `Failed to parse URI '${response}`;
+        errorPageURI = null;
+        Components.returnCode = Cr.NS_ERROR_ABORT;
+      }
+    }, e => {
+      errorPageURI = null;
+      Components.returnCode = Cr.NS_ERROR_ABORT;
     });
     Services.tm.spinEventLoopUntil(() =>
-        aWindow.closed || handled !== undefined);
+        aWindow.closed || errorPageURI !== undefined);
 
-    return handled || false;
+    return errorPageURI;
   }
 };