1254865 - remote-browser.xml's browser-child.js should not send a sync message on load

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

remote-browser.xml loads browser-child.js as a framescript, and one of the very last things that framescript does is send the sync Browser:Init message to the parent asking for some information as well as passing up the outerWindowID of the top-most window.

I think the sync message is only useful in that it tells browser-child whether or not the root docshell should use global history, and whether or not to init the AutoCompletePopup.

I suspect we can either send that information down either before or after browser-child.js loads, and without using sync messages.

Some ideas:

1) Send the information down when constructing the PBrowser on the parent side
2) Have the child receive this information in sync messages used to construct PBrowsers on the child side (like via PContent's createWindow).
3) Have the parent send the information down later in an async message
4) Make it possible to pass some arguments to a framescript when loadFrameScript is called, so that remote-browser.xml can send the information down at script loading time.

Getting rid of this sync message will help with the e10s tpaint regression.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 1254865 - Send disableglobalhistory state down to TabChild after construction asynchronously. r?smaug

disableglobalhistory is an attribute that we support on <xul:browser> that
can be used to signal to the underlying DocShell whether or not it should
record visits in global history.

This patch adds support for this attribute by detecting it at the time
that the TabParent is bound to the browser, and then sending the presence
of the attribute to the TabChild, which then sets the state in its DocShell.

Review commit: https://reviewboard.mozilla.org/r/42075/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42075/

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 1254865 - Don't send disableglobalhistory state down to browser-child.js in sync message. r?felipe

Review commit: https://reviewboard.mozilla.org/r/42089/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42089/

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 1254865 - Tests for disableglobalhistory on <xul:browser> elements. r?felipe

Review commit: https://reviewboard.mozilla.org/r/42087/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42087/

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 1254865 - Send init for AutoCompletePopup in async message from the parent. r?felipe

Review commit: https://reviewboard.mozilla.org/r/42077/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42077/

Comment 5

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

Comment on attachment 8734106 [details]
MozReview Request: Bug 1254865 - Send disableglobalhistory state down to TabChild after construction asynchronously. r?smaug

https://reviewboard.mozilla.org/r/42075/#review38613

Comment 6

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=136d118d12f6

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8734106 [details]
MozReview Request: Bug 1254865 - Send disableglobalhistory state down to TabChild after construction asynchronously. r?smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42075/diff/1-2/

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8734107 [details]
MozReview Request: Bug 1254865 - Don't send disableglobalhistory state down to browser-child.js in sync message. r?felipe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42089/diff/1-2/

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8734108 [details]
MozReview Request: Bug 1254865 - Tests for disableglobalhistory on <xul:browser> elements. r?felipe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42087/diff/1-2/

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8734109 [details]
MozReview Request: Bug 1254865 - Send init for AutoCompletePopup in async message from the parent. r?felipe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42077/diff/1-2/

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 1254865 - Set disableglobalhistory on the thumbnail browser. r?felipe

Review commit: https://reviewboard.mozilla.org/r/42339/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42339/

Comment 12

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8734109 [details]
MozReview Request: Bug 1254865 - Send init for AutoCompletePopup in async message from the parent. r?felipe

https://reviewboard.mozilla.org/r/42077/#review39131

Did you look at the history of the code here to figure out why AutoCompletePopup used a sync message?

Comment 13

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8734107 [details]
MozReview Request: Bug 1254865 - Don't send disableglobalhistory state down to browser-child.js in sync message. r?felipe

https://reviewboard.mozilla.org/r/42089/#review39133

Comment 14

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8734108 [details]
MozReview Request: Bug 1254865 - Tests for disableglobalhistory on <xul:browser> elements. r?felipe

https://reviewboard.mozilla.org/r/42087/#review39135

Comment 15

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8734532 [details]
MozReview Request: Bug 1254865 - Set disableglobalhistory on the thumbnail browser. r?felipe

https://reviewboard.mozilla.org/r/42339/#review39137

Comment 16

[Mike Conley (:mconley) (:⚙️)]

https://reviewboard.mozilla.org/r/42077/#review39131

I did. I couldn't find a reason listed in the bug where it got added (bug 897061). If it needed to wait for a tick of the event loop, it'll still get (at least) one since we'll be waiting for the async message from the parent.

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ae25970e4fc5
https://hg.mozilla.org/integration/mozilla-inbound/rev/f7affb27ef92
https://hg.mozilla.org/integration/mozilla-inbound/rev/358a4096b0ca
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2b8bb605d41
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c8a3acfb48c

Comment 18

[Joel Maher ( :jmaher ) (UTC -8)]

hey, this seems to have caused a perf win in tpaint!
https://treeherder.mozilla.org/perf.html#/alerts?id=653

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ae25970e4fc5
https://hg.mozilla.org/mozilla-central/rev/f7affb27ef92
https://hg.mozilla.org/mozilla-central/rev/358a4096b0ca
https://hg.mozilla.org/mozilla-central/rev/f2b8bb605d41
https://hg.mozilla.org/mozilla-central/rev/2c8a3acfb48c

Comment 20

[Mike Conley (:mconley) (:⚙️)]

(In reply to Joel Maher (:jmaher) (PTO- Back April 4th) from comment #18)
> hey, this seems to have caused a perf win in tpaint!
> https://treeherder.mozilla.org/perf.html#/alerts?id=653

That's the plan. :) Expect a few more in the next few days.

Comment 21

[:Felipe Gomes (needinfo for replies!)]

(In reply to Mike Conley (:mconley) - Needinfo me! from comment #20)
> (In reply to Joel Maher (:jmaher) (PTO- Back April 4th) from comment #18)
> > hey, this seems to have caused a perf win in tpaint!
> > https://treeherder.mozilla.org/perf.html#/alerts?id=653
> 
> That's the plan. :) Expect a few more in the next few days.

\o/

Comment 22

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bugnotes

http://people.mozilla.org/~mconley2/bugnotes/bug-1254865.html