fix(ios): return from safari webview goto once the new page has replaced the old one

devices/ios_webinspector.go

@@ -44,6 +44,8 @@ const (
 	safariVisibilityTimeout = 1 * time.Second
 
 	safariLoadStatePollEvery = 200 * time.Millisecond
+	// how long goto waits for the page it navigates to to replace the current one
+	safariNavigationTimeout = 30 * time.Second
 )
 
 var (
@@ -574,6 +576,7 @@ type safariWebViews struct {
 	// zero means the default; tests shorten them
 	findSafariTimeout time.Duration
 	livenessTimeout   time.Duration
+	navigateTimeout   time.Duration
 }
 
 func (s safariWebViews) findTimeout() time.Duration {
@@ -590,6 +593,13 @@ func (s safariWebViews) liveness() time.Duration {
 	return safariLivenessTimeout
 }
 
+func (s safariWebViews) navigationTimeout() time.Duration {
+	if s.navigateTimeout > 0 {
+		return s.navigateTimeout
+	}
+	return safariNavigationTimeout
+}
+
 // safariTabs returns Safari and the tabs it has open.
 func (s safariWebViews) safariTabs() (*webInspectorSession, inspectorApp, []inspectorPage, error) {
 	session, err := s.inspector.connected()
@@ -727,9 +737,30 @@ func (s safariWebViews) navigate(webviewID, body string, args []any) error {
 	return err
 }
 
-// WebKit has no command to navigate a page, so the page is told to do it.
+// safariGotoScript tells the page to navigate to arguments[0], and marks it with
+// arguments[1] first. The mark goes away with the page, so a page without it is
+// the one the navigation brought. A javascript: url runs in the page, and a jump
+// to a fragment keeps the page and changes its url at once, so neither replaces
+// the page and the mark is taken off again right away.
+const safariGotoScript = `var target = new URL(arguments[0], window.location.href);
+window.__mobilecliGoto = arguments[1];
+window.location.href = arguments[0];
+if (target.protocol === 'javascript:' || (target.href.indexOf('#') !== -1 && window.location.href === target.href)) delete window.__mobilecliGoto;`
+
+// WebKit has no command to navigate a page, so the page is told to do it. The
+// navigation only starts there, and a wait right after it would still find the
+// page being left, so goto returns once the new page has replaced it.
 func (s safariWebViews) WebViewGoto(webviewID, url string) error {
-	return s.navigate(webviewID, "window.location.href = arguments[0]", []any{url})
+	mark, err := newInspectorID()
+	if err != nil {
+		return err
+	}
+	if err := s.navigate(webviewID, safariGotoScript, []any{url, mark}); err != nil {
+		return err
+	}
+	isNewPage := fmt.Sprintf("return window.__mobilecliGoto !== %q", mark)
+	timedOut := fmt.Sprintf("navigation to %s did not replace the page within %s", url, s.navigationTimeout())
+	return s.waitUntil(webviewID, isNewPage, s.navigationTimeout(), timedOut)
 }
 
 func (s safariWebViews) WebViewGoBack(webviewID string) error {
@@ -814,41 +845,49 @@ func (s safariWebViews) WebViewWaitForLoadState(webviewID, state string, timeout
 		hasLoaded = "return document.readyState === 'interactive' || document.readyState === 'complete'"
 	}
 
-	deadline := time.Now().Add(time.Duration(timeoutMs) * time.Millisecond)
+	timedOut := fmt.Sprintf("waitForLoadState timed out waiting for '%s'", state)
+	return s.waitUntil(webviewID, hasLoaded, time.Duration(timeoutMs)*time.Millisecond, timedOut)
+}
+
+// waitUntil evaluates condition in the tab until it returns true, for at most
+// timeout. Running out of time fails with timedOut, followed by why the last
+// poll failed if it did.
+func (s safariWebViews) waitUntil(webviewID, condition string, timeout time.Duration, timedOut string) error {
+	deadline := time.Now().Add(timeout)
 	for {
 		// a poll on a tab that does not answer must not outlive the wait
 		pollTimeout := min(time.Until(deadline), s.liveness())
 		if pollTimeout <= 0 {
-			return fmt.Errorf("waitForLoadState timed out waiting for '%s'", state)
+			return errors.New(timedOut)
 		}
 		// the tab does not answer while it swaps pages mid-navigation, so only
 		// an unknown tab ends the wait early
-		loaded, err := s.hasLoaded(webviewID, hasLoaded, pollTimeout)
+		holds, err := s.holds(webviewID, condition, pollTimeout)
 		if errors.Is(err, errSafariTabNotFound) {
 			return err
 		}
-		if err == nil && loaded {
+		if err == nil && holds {
 			return nil
 		}
 		if time.Now().After(deadline) {
 			if err != nil {
-				return fmt.Errorf("waitForLoadState timed out waiting for '%s': %w", state, err)
+				return fmt.Errorf("%s: %w", timedOut, err)
 			}
-			return fmt.Errorf("waitForLoadState timed out waiting for '%s'", state)
+			return errors.New(timedOut)
 		}
 		time.Sleep(safariLoadStatePollEvery)
 	}
 }
 
-// hasLoaded attaches to the tab and evaluates hasLoaded in it, within timeout in all.
-func (s safariWebViews) hasLoaded(webviewID, hasLoaded string, timeout time.Duration) (bool, error) {
+// holds attaches to the tab and evaluates condition in it, within timeout in all.
+func (s safariWebViews) holds(webviewID, condition string, timeout time.Duration) (bool, error) {
 	deadline := time.Now().Add(timeout)
 	tab, err := s.attach(webviewID, timeout)
 	if err != nil {
 		return false, err
 	}
 	defer tab.detach()
 
-	loaded, err := tab.evaluate(hasLoaded, nil, time.Until(deadline))
-	return loaded == true, err
+	result, err := tab.evaluate(condition, nil, time.Until(deadline))
+	return result == true, err
 }

devices/ios_webinspector_test.go

@@ -573,26 +573,120 @@ func waitUntilTheClientNoticed(t *testing.T, client *webInspectorClient) {
 	}
 }
 
+// pageIsReplacedAfter makes the tab answer polls for its page as if the page
+// were replaced by the new one on the given poll. Only the polls await a
+// promise: the navigation itself is not awaited.
+func (f *fakeWebInspector) pageIsReplacedAfter(polls int) *int {
+	polled := 0
+	f.answer = func(call fakeInspectorCall) (any, string) {
+		if call.Method != "Runtime.awaitPromise" {
+			return promiseReference, ""
+		}
+		polled++
+		return map[string]any{"result": map[string]any{"type": "boolean", "value": polled >= polls}, "wasThrown": false}, ""
+	}
+	return &polled
+}
+
+// expressionOf returns the javascript of the nth evaluation the tab was sent
+func expressionOf(inspector *fakeWebInspector, nth int) string {
+	expression, _ := inspector.callsOf("Runtime.evaluate")[nth].Params["expression"].(string)
+	return expression
+}
+
+func navigationOf(inspector *fakeWebInspector) string {
+	return expressionOf(inspector, 0)
+}
+
 // WebKit has no command to navigate a page, so the page is told to navigate
 // itself. The url travels as an argument, not as javascript source.
 func TestSafariGotoNavigatesTheTabToTheUrl(t *testing.T) {
 	inspector := safariShowing(t, examplePage)
+	inspector.pageIsReplacedAfter(1)
 
 	err := inspector.safari().WebViewGoto("2", "https://mobilewright.dev/?q='quoted'")
 
 	if err != nil {
 		t.Fatalf("WebViewGoto: %v", err)
 	}
-	expression := inspector.onlyCallOf("Runtime.evaluate").Params["expression"].(string)
-	if expression != "(function(){\nwindow.location.href = arguments[0]\n}).apply(null, [\"https://mobilewright.dev/?q='quoted'\"])" {
-		t.Fatalf("unexpected expression %q", expression)
+	navigation := navigationOf(inspector)
+	if !strings.HasPrefix(navigation, "(function(){\n"+safariGotoScript+"\n}).apply(null, [\"https://mobilewright.dev/?q='quoted'\",\"") {
+		t.Fatalf("expected the url to be passed as an argument, evaluated %q", navigation)
+	}
+}
+
+// A wait issued right after goto must not find the page goto navigated away
+// from, so goto returns only once the new page has replaced it.
+func TestSafariGotoReturnsOnceTheNewPageHasReplacedTheOldOne(t *testing.T) {
+	inspector := safariShowing(t, examplePage)
+	polled := inspector.pageIsReplacedAfter(3)
+
+	err := inspector.safari().WebViewGoto("2", "https://mobilewright.dev/")
+
+	if err != nil {
+		t.Fatalf("WebViewGoto: %v", err)
+	}
+	if *polled != 3 {
+		t.Fatalf("expected goto to poll until the page was replaced, polled %d times", *polled)
+	}
+}
+
+// The page goto leaves is marked, and the page that has no mark is the new one.
+func TestSafariGotoRecognizesTheNewPageByTheMarkItLeftOnTheOldOne(t *testing.T) {
+	inspector := safariShowing(t, examplePage)
+	inspector.pageIsReplacedAfter(1)
+
+	if err := inspector.safari().WebViewGoto("2", "https://mobilewright.dev/"); err != nil {
+		t.Fatalf("WebViewGoto: %v", err)
+	}
+
+	_, markArgument, _ := strings.Cut(navigationOf(inspector), "[\"https://mobilewright.dev/\",")
+	mark := strings.TrimSuffix(markArgument, "])")
+	poll := expressionOf(inspector, 1)
+	if mark == "" || !strings.Contains(poll, "return window.__mobilecliGoto !== "+mark) {
+		t.Fatalf("expected the poll to look for the absence of mark %s, evaluated %q", mark, poll)
+	}
+}
+
+func TestSafariGotoUsesANewMarkForEachNavigation(t *testing.T) {
+	inspector := safariShowing(t, examplePage)
+	inspector.pageIsReplacedAfter(1)
+	safari := inspector.safari()
+
+	if err := safari.WebViewGoto("2", "https://mobilewright.dev/"); err != nil {
+		t.Fatalf("WebViewGoto: %v", err)
+	}
+	if err := safari.WebViewGoto("2", "https://mobilewright.dev/"); err != nil {
+		t.Fatalf("WebViewGoto: %v", err)
+	}
+
+	if first, second := expressionOf(inspector, 0), expressionOf(inspector, 2); first == second {
+		t.Fatalf("expected each navigation to mark the page differently, both evaluated %q", first)
+	}
+}
+
+func TestSafariGotoFailsWhenTheNavigationNeverReplacesThePage(t *testing.T) {
+	inspector := safariShowing(t, examplePage)
+	inspector.evaluatesTo(false)
+	safari := inspector.safari()
+	safari.navigateTimeout = 300 * time.Millisecond
+
+	start := time.Now()
+	err := safari.WebViewGoto("2", "http://localhost:1/")
+
+	if elapsed := time.Since(start); elapsed > 1500*time.Millisecond {
+		t.Fatalf("goto took %s for a 300ms timeout", elapsed)
+	}
+	if err == nil || err.Error() != "navigation to http://localhost:1/ did not replace the page within 300ms" {
+		t.Fatalf("expected goto to fail saying the page was not replaced, got %v", err)
 	}
 }
 
 // The page that answers is torn down by the navigation it starts, so nothing
 // may be awaited from it afterwards.
 func TestSafariNavigationDoesNotAwaitThePageItLeaves(t *testing.T) {
 	inspector := safariShowing(t, examplePage)
+	inspector.pageIsReplacedAfter(1)
 	safari := inspector.safari()
 
 	if err := safari.WebViewGoto("2", "https://mobilewright.dev/"); err != nil {
@@ -605,8 +699,10 @@ func TestSafariNavigationDoesNotAwaitThePageItLeaves(t *testing.T) {
 		t.Fatalf("WebViewGoForward: %v", err)
 	}
 
-	if awaited := inspector.callsOf("Runtime.awaitPromise"); len(awaited) != 0 {
-		t.Fatalf("expected no promise to be awaited, got %v", awaited)
+	// goto awaits its one poll of the new page, and nothing of the pages it and
+	// the history moves leave
+	if awaited := inspector.callsOf("Runtime.awaitPromise"); len(awaited) != 1 {
+		t.Fatalf("expected only the poll of the new page to be awaited, got %v", awaited)
 	}
 }