Bug 1447451 - Correctly report page progress errors in GeckoView r=esawin
authorJames Willcox <snorp@snorp.net>
Tue, 20 Mar 2018 15:55:36 -0500
changeset 409103 cd617afab319afc4d9edf5ecceb052a735f667f9
parent 409102 9c7e42b18aecf22a9f93ca850839d7e7eb79d7e7
child 409104 84c7300864d7b6dc9719cbba9dbabbe0e7a1e6a9
push id33675
push usertoros@mozilla.com
push dateWed, 21 Mar 2018 09:40:24 +0000
treeherdermozilla-central@f4ddf30ecf57 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersesawin
bugs1447451
milestone61.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 1447451 - Correctly report page progress errors in GeckoView r=esawin This is a far cry from having good error reporting, but we at least need to notify the app that the page is no longer loading. MozReview-Commit-ID: Kbm5MpNbND6
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ProgressDelegateTest.kt
mobile/android/modules/geckoview/GeckoViewProgress.jsm
--- 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
@@ -46,16 +46,46 @@ class ProgressDelegateTest : BaseSession
             @AssertCalled(count = 1, order = intArrayOf(3))
             override fun onPageStop(session: GeckoSession, success: Boolean) {
                 assertThat("Session should not be null", session, notNullValue())
                 assertThat("Load should succeed", success, equalTo(true))
             }
         })
     }
 
+    fun loadExpectNetError(testUri: String) {
+        sessionRule.session.loadUri(testUri);
+        sessionRule.waitForPageStop()
+
+        sessionRule.forCallbacksDuringWait(object : Callbacks.ProgressDelegate, Callbacks.NavigationDelegate {
+            @AssertCalled(count = 2)
+            override fun onLoadRequest(session: GeckoSession, uri: String, where: Int): Boolean {
+                if (sessionRule.currentCall.counter == 1) {
+                    assertThat("URI should be " + testUri, uri, equalTo(testUri));
+                } else {
+                    assertThat("URI should be about:neterror", uri, startsWith("about:neterror"));
+                }
+                return false
+            }
+
+            @AssertCalled(count = 1)
+            override fun onPageStop(session: GeckoSession, success: Boolean) {
+                assertThat("Load should fail", success, equalTo(false))
+            }
+        })
+    }
+
+    @Test fun loadUnknownHost() {
+        loadExpectNetError("http://does.not.exist.mozilla.org/")
+    }
+
+    @Test fun loadBadPort() {
+        loadExpectNetError("http://localhost:1/")
+    }
+
     @Test fun multipleLoads() {
         sessionRule.session.loadUri(INVALID_URI)
         sessionRule.session.loadTestPath(HELLO_HTML_PATH)
         sessionRule.waitForPageStops(2)
 
         sessionRule.forCallbacksDuringWait(object : Callbacks.ProgressDelegate {
             @AssertCalled(count = 2, order = intArrayOf(1, 3))
             override fun onPageStart(session: GeckoSession, url: String) {
--- a/mobile/android/modules/geckoview/GeckoViewProgress.jsm
+++ b/mobile/android/modules/geckoview/GeckoViewProgress.jsm
@@ -217,42 +217,46 @@ class GeckoViewProgress extends GeckoVie
     let settings = this.settings;
     debug("onSettingsUpdate: " + JSON.stringify(settings));
 
     IdentityHandler.setUseTrackingProtection(!!settings.useTrackingProtection);
     IdentityHandler.setUsePrivateMode(!!settings.usePrivateMode);
   }
 
   onStateChange(aWebProgress, aRequest, aStateFlags, aStatus) {
-    debug("onStateChange()");
+    debug(`onStateChange() isTopLevel=${aWebProgress.isTopLevel}, stateFlags=${aStateFlags}, state=${aStatus}`);
 
     if (!aWebProgress.isTopLevel) {
       return;
     }
 
+    const uriSpec = aRequest.QueryInterface(Ci.nsIChannel).URI.displaySpec;
+    debug(`onStateChange() URI=${uriSpec}`);
+
     if (aStateFlags & Ci.nsIWebProgressListener.STATE_START) {
-      let uri = aRequest.QueryInterface(Ci.nsIChannel).URI;
-      let message = {
+      const message = {
         type: "GeckoView:PageStart",
-        uri: uri.displaySpec,
+        uri: uriSpec,
       };
 
       this.eventDispatcher.sendRequest(message);
     } else if ((aStateFlags & Ci.nsIWebProgressListener.STATE_STOP) &&
                !aWebProgress.isLoadingDocument) {
       let message = {
         type: "GeckoView:PageStop",
         success: !aStatus
       };
 
       this.eventDispatcher.sendRequest(message);
     }
   }
 
   onSecurityChange(aWebProgress, aRequest, aState) {
+    debug("onSecurityChange()");
+
     // Don't need to do anything if the data we use to update the UI hasn't changed
     if (this._state === aState && !this._hostChanged) {
       return;
     }
 
     this._state = aState;
     this._hostChanged = false;
 
@@ -262,11 +266,20 @@ class GeckoViewProgress extends GeckoVie
       type: "GeckoView:SecurityChanged",
       identity: identity
     };
 
     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
+      });
+    }
   }
 }