Skip to content

Commit

Permalink
[soft navigations] Refactor navigation heuristics
Browse files Browse the repository at this point in the history
This CL refactors the navigation heuristics and related API, simplifying
the requirements and data flow.

URL change detection
-------------------------
Previously, soft nav heuristics tracked both a "has-url-change" bit and
a string URL, initially empty. Both needed to be set to be considered a
soft nav, which is redundant. This CL removes the "has-url-change" bit,
set in SameDocumentNavigationStarted(), relying solely on the string URL
set in SameDocumentNavigationCommitted().

Additionally, the relevant SoftNavigationContext is passed to
SameDocumentNavigationCommitted() to simplify the data flow. For metrics
purposes, calling SameDocumentNavigationCommitted() with a null context
records SoftNavigationOutcome::kNoAncestorTask (previously this was
recording in SameDocumentNavigationStarted()).

Context propagation
-------------------------
For async same-document navigations, i.e. renderer -> browser ->
renderer, the navigation task state must be propagated to the commit so
it can be restored for the events (popstate). This works the same as before, except:
 - SameDocumentNavigationStarted() is renamed to
   AsyncSameDocumentNavigationStarted(), and it's only called for async
   navigations.
 - AsyncSameDocumentNavigationStarted() handles registering the current
   task with the task attribution tracker.
 - A bug was fixed where the propagated context could be lost in the
   navigation API plumbing, which was necessary for consistency of the
   popstate event as well as being able to pass the relevant context to
   SameDocumentNavigationCommitted().

Tests
-------------------------
 - A unit test is added to ensure we don't accidentally emit the same
   soft nav more than once. A previous version of this CL was doing
   that, and it was only accidentally caught by a bug in another test
   (below).
 - This fixes a web components soft-nav WPT test. The pushState in the
   connected callback was not having an effect since the test already
   called pushState. This was found during testing.

Bug: 40942324
Change-Id: Ic7723251e8f7b277d0572f29ae0c2ea8d400c1bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5458385
Reviewed-by: Yoav Weiss (@Shopify) <yoavweiss@chromium.org>
Commit-Queue: Scott Haseley <shaseley@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Michal Mocny <mmocny@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1301456}
  • Loading branch information
shaseley authored and chromium-wpt-export-bot committed May 15, 2024
1 parent 4c4bfc3 commit bc6f2e3
Showing 1 changed file with 1 addition and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
main.appendChild(spaContent);
},
link: link,
pushState: null,
test: "Test that a soft navigation is detected when the click is done "
+ "on a custom element."});
</script>
Expand Down

0 comments on commit bc6f2e3

Please sign in to comment.