For Fission, we need to make session history navigations (history.back, forward, go(), etc.) asynchronous.

A simple change (but almost what the spec says, but the spec may be totally broken here).
Let's see how many tests are broken
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f278651f77a84e18939600e67111fe7c75a8e143

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c3bea4366a56beb77fbd242dcc03beb63f10bc8

And to clarify, this bug is not about popstate handling. That is rather different thing and handled in bug 1281952.
Quite a few of the wpt tests fail because of sync popstate.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9dd2d4f4244247b46ca2a067050efcf0d6b259a7
(Trying to make GeckoView's fragment navigation less broken in general)

The main part of the change is the change to ChildSHistory - make it possible to have Go() to be called asynchronously
and also let one to cancel pending history navigations. History object (window.history) can then use either the sync or
async Go(), depending on the dom.window.history.async pref.

LoadDelegate, which is used by GeckoView, needs special handling, since
it spins event loop nestedly. With session history loads and same-document loads we can just
bypass it.
To deal with same-document case, MaybeHandleSameDocumentNavigation is split to IsSameDocumentNavigation,
which collects relevant information about the request and returns true if same-document navigation should happen,
and then later HandleSameDocumentNavigation uses that information to trigger the navigation.
SameDocumentNavigationState is used to pass the information around.

Enabling the pref explicitly in dir.ini so that if we need to disable the pref on Nightly, we can
still run the tests.

(Need to update the patch, I was missing making some parts async :/)

Backed out 2 changesets (bug 1563587) for junit failure

Backout: https://hg.mozilla.org/integration/autoland/rev/8d2d3b3fb7e27f0bd238985c265b4f50dc10bc05

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=a365d3c4326127be0d85de3c7027cfd4174a4177

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=261401491&repo=autoland&lineNumber=1779

https://hg.mozilla.org/mozilla-central/rev/214fac6eb1c0

https://hg.mozilla.org/mozilla-central/rev/fcd47d1e1ed5

https://hg.mozilla.org/mozilla-central/rev/3e0fffc6007d

This is not just a refactor; it may affect websites that relied on the synchronous behavior (which would be incompatible with the web).

For example:

history.pushState(1, "");
history.pushState(2, "");
history.back();
console.log(history.state);
history.forward();
console.log(history.state);

// Result (Firefox 69): "1", "2" and returned to the initial history entry
// Result (Firefox 70, Chromium 76, Safari 12.1.2): "2", "2" and ends at the previous history entry (history.forward did not appear to work).

Shouldn't this change be listed on MDN (dev-doc-needed) or be added to https://www.fxsitecompat.dev ?

I didn't look at the full patch in detail, but at the top of the patch setTimeout was added after history.back(). Shouldn't the popstate event be used instead, to make sure that the history navigation has been processed?

(In reply to Rob Wu [:robwu] from comment #26)

Shouldn't this change be listed on MDN (dev-doc-needed) or be added to https://www.fxsitecompat.dev ?

yes

I didn't look at the full patch in detail, but at the top of the patch setTimeout was added after history.back(). Shouldn't the popstate event be used instead, to make sure that the history navigation has been processed?

I don't know which setTimeout you're talking about. In some cases popstate would have worked, in some case not.
And wpt tests got modified some more on github.

(In reply to Olli Pettay [:smaug] from comment #27)

(In reply to Rob Wu [:robwu] from comment #26)

I didn't look at the full patch in detail, but at the top of the patch setTimeout was added after history.back(). Shouldn't the popstate event be used instead, to make sure that the history navigation has been processed?

I don't know which setTimeout you're talking about. In some cases popstate would have worked, in some case not.
And wpt tests got modified some more on github.

The top of [ the patch ] ( https://hg.mozilla.org/mozilla-central/rev/214fac6eb1c0 ), i.e.

--- a/browser/base/content/test/sanitize/browser_purgehistory_clears_sh.js
+++ b/browser/base/content/test/sanitize/browser_purgehistory_clears_sh.js
@@ -27,16 +27,19 @@ add_task(async function purgeHistoryTest
       ok(backButton.hasAttribute("disabled"), "Back button is disabled");
       ok(forwardButton.hasAttribute("disabled"), "Forward button is disabled");
 
       await ContentTask.spawn(browser, null, async function() {
         let startHistory = content.history.length;
         content.history.pushState({}, "");
         content.history.pushState({}, "");
         content.history.back();
+        await new Promise(function(r) {
+          setTimeout(r);
+        });

... but also the many (indirect) setTimeouts that are called after history.back or history.forward. I would expect those tests to fail intermittently if the implementation of history navigation changes (to the extent that it takes longer than one macrotask).

Just to make confirm before I make the change that I'm not missing something important. Is the main change (where the docs are concerned) that history.back, history.forward, and history.go are no asynchronous and return a promise?

No Promises around anywhere.
The API just work asynchronously.

They are asynchronous indeed, but they don't return a promise. To detect completion of history.back/forward/go, the popstate event has to be used instead.

(In reply to Rob Wu [:robwu] from comment #31)

They are asynchronous indeed, but they don't return a promise. To detect completion of history.back/forward/go, the popstate event has to be used instead.

Thank you. I'm not terribly good with asynchronous programming. I can't seem to get it into my head properly. I though that in JavaScript, asynchronous implied promises.

Update made to the go, back, and forward methods to include the information that the methods are now asynchronous. Also added a statement to the effect to the Firefox 70 release notes:

The back(), forward(), and go() methods are now asynchronous. Add a listener to the popstate event to get notification that navigation has completed bug 1563587.

Posted site compatibility note: https://www.fxsitecompat.dev/en-CA/docs/2019/history-navigation-methods-are-now-asynchronous/

There might be need for this on esr68

esr68 try
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc64dc5b70a40d86b2ecf33b13d43f86dbd4c834

Apparently tryserver had some issues unrelated to the patch
https://treeherder.mozilla.org/#/jobs?repo=try&revision=00f2f8f61c907b529e6462a8c34bd12182562cca

And need also bug 1396309 for esr

Comment on attachment 9114908 [details]
Bug 1563587, Make history.back/forward/go asynchronous, esr68, r=farre

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: To have consistent history.* handling also in ESR, and especially because of
  https://bugzilla.mozilla.org/show_bug.cgi?id=1528587#c45
• User impact if declined: See bug 1528587
• Fix Landed on Version: 70
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Changes to history API behavior tend to cause regressions.
  The original patch did regress wikipedia (largely because they had Chrome-only code to deal with asynchronousness).
• String or UUID changes made by this patch:

Is there a chance this will affect fennec particularly when we uplift? (Any specific testing we can/should do around that?)

The patch would affect all 68. But if we want to excluded Fennec, dom.window.history.async could be set to false there.

Comment on attachment 9114908 [details]
Bug 1563587, Make history.back/forward/go asynchronous, esr68, r=farre

I discussed this with Dan and don't think this risk of taking this patch (with at least one known regression since it shipped on release users) is justified against the severity of the bug being fixed.

This bug appears to have caused a regression - https://bugzilla.mozilla.org/show_bug.cgi?id=1605556