commit 6c5f39bd1022b07dcc600f34de592f9d40873a9b
author Hiroki Nakagawa <nhiroki@chromium.org> Tue Jul 13 00:22:36 2021
committer Chromium LUCI CQ <chromium-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Jul 13 00:22:36 2021
tree f49bf72ca1138541264b1772835745f6f9068fb6
parent c2973580843dd74dc176d2cda4c8ec67f940c631

Prerender: Make prerender activation synchronous

To avoid prerendering from being cancelled during prerender activation,
this CL makes the prerender activation sequence (BeginNavigationImpl to
DidFinishNavigation) synchronous.

This CL takes 2 approaches:

1) remove asynchronous operations during activation.

   This CL removes PostTask from CachedNavigationURLLoader.

2) moves asynchronous operations during activation to the beginning of
   navigation start where it is still feasible to fall back to regular
   navigation.

   This CL runs CommitDeferringConditionRunner at the beginning of
   navigation start instead of before commit.

With these changes, prerender activation can synchronously finish after
all CommitDeferringConditions run.

> Activation flow

- NavigationRequest::BeginNavigation() looks up a reservable
  PrerenderHost in PrerenderHostRegistry. If the host is found,
  BeginNavigation() starts running CommitDeferringConditions. These
  conditions may defer navigation, that is, these can be asynchronous,
  but it's ok because the navigation request doesn't interact with the
  host at this point and can easily fall back to regular navigation.
  After all the conditions run,
  OnCommitDeferringConditionChecksComplete() is called.
- In OnCommitDeferringConditionChecksComplete(),
  - if the host is still reservable, the navigation request reserves the
    host and proceeds with the activation. The rest of the activation
    sequence synchronously finishes, so it is expected that no one
    requests cancellation during the period. The commit deferring
    conditions don't run again before commit.
  - if the host is no longer reservable for some reasons (e.g.,
    prerendering is cancelled), the navigation request gives up the
    activation and starts regular navigation. The commit deferring
    conditions runs again before commit as usual.

> Design

- Some of changes in this CL could also applicable to restoration from
  BackForwardCache (e.g., making CachedNavigationURLLoader synchronous),
  but this CL doesn't do that to avoid blocking Prerender by unexpected
  breakages in BackForwardCache.
- This CL changes the timing to register CommitDeferringConditions with
  MockCommitDeferringConditionInstaller. Before this CL, the installer
  used WebContentsObserver::DidStartNavigation() to inject conditions.
  After this CL, this timing is too late because the conditions run in
  BeginNavigation() that is earlier than DidStartNavigation(). After
  this CL, the installer injects conditions when
  CommitDeferringConditionRunner::RegisterDeferringCondition() is
  called before starting the runner.
- For prerendered page activation, WebContentsObserver calls for
  navigation (i.e., DidStartNavigation, ReadyToCommitNavigation, and
  DidFinishNavigation) are expected to synchronously run. This CL adds
  TaskChecker to ensure they run in the same task. The checker is used
  in WebContentsObserverConsistencyChecker.
- See also the discussion on navigation-dev[1]. We also had discussed
  other approach: restarting navigation when prerendering is cancelled
  during activation. However, we did not adopt this approach because
  it turned out that both creating a new NavigationRequest and
  resetting the current NavigationRequest for restart navigation are
  difficult (see the design docs [2, 3]).

[1]https://groups.google.com/a/chromium.org/g/navigation-dev/c/MWAiXKIiXP0/m/ZB8xXAAmAAAJ
[2]https://docs.google.com/document/d/1B2ljj53rMHKCOCyWV3ekpul8CR4BsIxOZdaK4kXV8pM/edit?usp=sharing
[3]https://docs.google.com/document/d/1kaZd_MbDWxaRUycEmpw4hq9-QpFpr2znuez2WmYRUM8/edit?usp=sharing

Change-Id: Iead6736814d0ef5b3c28c96e7ee9421b061808f8
Bug: 1195751
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2992411
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900746}