1383876 - Remove remaining link "prerender" code

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Nika Layzell [:nika] (ni? for response)]

We don't intend to finish the implementation which was added in bug 1315105, so we should remove the code which was added in that bug and related bugs.

However, we do want to keep the GroupedSHistory work which :freesamael worked on, as we may want to extend it in the future to improve cross-process loads in e10s-multi. 

This bug is to track removing the rest of that code.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

This was never shipped or documented anywhere as far as I know - I doubt we'll need any dev-doc work.

Comment 2

[Chris Mills [:cmills]]

(In reply to Michael Layzell [:mystor] from comment #1)
> This was never shipped or documented anywhere as far as I know - I doubt
> we'll need any dev-doc work.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1315105#c63

Comment 3

[Nika Layzell [:nika] (ni? for response)]

I'm taking this to remove this unused code to start working towards bug 1419857.

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Remove GroupedSHistory and Prerendering logic from C++ code

This is the first part of removing as much of the prerendering logic as I could
find from the code.

It would be good to go over this & make sure that we actually wanted to remove
everything which I remove here. The second patch also removes a bunch of tests
which we also need to make sure we actually want to remove.

If you notice something I totaly missed let me know.

I'm doing a try push with this right now.

MozReview-Commit-ID: 2aHA6NcQPGk

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 2: Remove GroupedSHistory and Prerendering logic from JS code

MozReview-Commit-ID: KwvOcgQKheK

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 3: Remove prerendering methods from nsIWebBrowserChrome3.idl

I found more ^.^

MozReview-Commit-ID: IuJHv3n0Uob

Comment 7

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 4: Remove even more tests which no longer work

MozReview-Commit-ID: 5j8fNeKuHAM

Comment 8

[Samael Wang [:freesamael] (away for now)]

Comment on attachment 8936944 [details] [diff] [review]
Part 1: Remove GroupedSHistory and Prerendering logic from C++ code

Review of attachment 8936944 [details] [diff] [review]:
-----------------------------------------------------------------

I tried my best...

::: docshell/shistory/nsSHistory.cpp
@@ +1651,1 @@
>    } else {

Remove the `else` since it early returns above.

::: dom/base/nsFrameLoader.cpp
@@ +364,2 @@
>  bool
>  nsFrameLoader::SwapBrowsersAndNotify(nsFrameLoader* aOther)

Do you plan to keep `BrowserWillChangeProcess` & `BrowserChangedProcess` stuff? Otherwise this can be removed I think.

@@ +401,2 @@
>  already_AddRefed<Promise>
>  nsFrameLoader::FireWillChangeProcessEvent()

Same here.

@@ +437,2 @@
>  void
>  nsFrameLoader::AddProcessChangeBlockingPromise(Promise& aPromise, ErrorResult& aRv)

And here. And the whole `GroupedHistoryEvent` class + webidl.

::: dom/base/nsFrameLoader.h
@@ +468,3 @@
>    // A stack-maintained reference to an array of promises which are blocking
>    // grouped history navigation
>    nsTArray<RefPtr<mozilla::dom::Promise>>* mBrowserChangingProcessBlockers;

Same here.

::: dom/ipc/TabChild.cpp
@@ +3510,2 @@
>  already_AddRefed<nsISHistory>
>  TabChild::GetRelatedSHistory()

Looks like we removed all its callers.

::: dom/webidl/FrameLoader.webidl
@@ -12,2 @@
>  interface nsIMessageSender;
> -interface nsIPartialSHistory;

We can remove these 2 bindings:
https://searchfox.org/mozilla-central/rev/110706c3c09d457dc70293b213d7bccb4f6f5643/dom/bindings/Bindings.conf#1742-1743,1750-1751

::: dom/xul/nsXULElement.cpp
@@ -1491,5 @@
>          (new AsyncEventDispatcher(this,
>                                    NS_LITERAL_STRING("XULFrameLoaderCreated"),
>                                    /* aBubbles */ true))->RunDOMEventWhenSafe();
> -
> -        if (AttrValueIs(kNameSpaceID_None, nsGkAtoms::prerendered,

And the GkAtoms. 

Do you happen to know how the rust atoms mapping are generated? We may want to also remove this one:
https://searchfox.org/mozilla-central/rev/110706c3c09d457dc70293b213d7bccb4f6f5643/servo/components/style/gecko/generated/atom_macro.rs#2220

Comment 9

[Samael Wang [:freesamael] (away for now)]

Comment on attachment 8936949 [details] [diff] [review]
Part 3: Remove prerendering methods from nsIWebBrowserChrome3.idl

Review of attachment 8936949 [details] [diff] [review]:
-----------------------------------------------------------------

One missing implementation:
https://searchfox.org/mozilla-central/rev/110706c3c09d457dc70293b213d7bccb4f6f5643/toolkit/components/extensions/ext-browser-content.js#315-320

Comment 10

[Nika Layzell [:nika] (ni? for response)]

FYI: I want to keep around the GroupedHistoryEvent, BrowserChangedProcess, BrowserWillChangeProcess and the process blockers, because I'm going to want to use them in the future.

I will probably rename GroupedHistoryEvent.

Comment 11

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Remove GroupedSHistory and Prerendering logic from C++ code

MozReview-Commit-ID: 2aHA6NcQPGk

Comment 12

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 2: Remove GroupedSHistory and Prerendering logic from JS code

MozReview-Commit-ID: KwvOcgQKheK

Comment 13

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 3: Remove prerendering methods from nsIWebBrowserChrome3.idl

MozReview-Commit-ID: IuJHv3n0Uob

Comment 14

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 4: Remove even more tests which no longer work

MozReview-Commit-ID: 5j8fNeKuHAM

Comment 15

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 5: Clean up in moz.build files

MozReview-Commit-ID: 1HKNAeI7hSV

Comment 16

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8937127 [details] [diff] [review]
Part 1: Remove GroupedSHistory and Prerendering logic from C++ code

I forgot that Samael isn't a DOM peer (and neither am I), so I'm going to need you to sign off on my webidl changes.

Nothing web visible has been changed.

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

So we don't want to use this stuff to handle process switches properly? Or do we not want to switch process ever for a tab?

Comment 18

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8937127 [details] [diff] [review]
Part 1: Remove GroupedSHistory and Prerendering logic from C++ code

r+ for the .webidl.

Comment 19

[Nika Layzell [:nika] (ni? for response)]

I went through this code again, and I'm pretty sure we don't want any of this stuff even if we want to have bfcache in the first version of improved process switching.

I've updated the patch to apply on the new tree & I'm going to land it.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/156c6bee1752
Part 1: Remove GroupedSHistory and Prerendering logic from C++ code, r=freesamael, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/156088fcf75b
Part 2: Remove GroupedSHistory and Prerendering logic from JS code, r=freesamael
https://hg.mozilla.org/integration/mozilla-inbound/rev/e89b001fd9f5
Part 3: Remove prerendering methods from nsIWebBrowserChrome3.idl, r=freesamael
https://hg.mozilla.org/integration/mozilla-inbound/rev/c61976e2d3ac
Part 4: Remove even more tests which no longer work, r=freesamael, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d9855f7bfdb
Part 5: Clean up in moz.build files, r=freesamael

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0683e2979c0
Part 6: Fix eslint failures, a=bustage

Comment 22

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/156c6bee1752
https://hg.mozilla.org/mozilla-central/rev/156088fcf75b
https://hg.mozilla.org/mozilla-central/rev/e89b001fd9f5
https://hg.mozilla.org/mozilla-central/rev/c61976e2d3ac
https://hg.mozilla.org/mozilla-central/rev/6d9855f7bfdb
https://hg.mozilla.org/mozilla-central/rev/e0683e2979c0