Closed Bug 1254865 Opened 8 years ago Closed 8 years ago

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

Categories

(Toolkit :: UI Widgets, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: mconley, Assigned: mconley)

References

Details

Attachments

(6 files)

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.
Blocks: 1174239
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/
Attachment #8734106 - Flags: review?(bugs)
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
Attachment #8734106 - Flags: review?(bugs) → review+
Depends on: 1259574
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 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 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 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 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?
Attachment #8734109 - Flags: review?(felipc) → review+
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
Attachment #8734107 - Flags: review?(felipc) → review+
Attachment #8734108 - Flags: review?(felipc) → review+
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
Attachment #8734532 - Flags: review?(felipc) → review+
Comment on attachment 8734532 [details]
MozReview Request: Bug 1254865 - Set disableglobalhistory on the thumbnail browser. r?felipe

https://reviewboard.mozilla.org/r/42339/#review39137
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.
hey, this seems to have caused a perf win in tpaint!
https://treeherder.mozilla.org/perf.html#/alerts?id=653
(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.
(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/
Depends on: 1259914
Depends on: 1261599
Blocks: 1264402
Assignee: nobody → mconley
No longer blocks: 1264402
Depends on: 1267844
You need to log in before you can comment on or make changes to this bug.