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)
Toolkit
UI Widgets
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: mconley, Assigned: mconley)
References
Details
Attachments
(6 files)
58 bytes,
text/x-review-board-request
|
smaug
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
Felipe
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
Felipe
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
Felipe
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
Felipe
:
review+
|
Details |
6.97 KB,
application/zip
|
Details |
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.
Assignee | ||
Comment 1•8 years ago
|
||
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)
Assignee | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/42089/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42089/
Attachment #8734107 -
Flags: review?(felipc)
Assignee | ||
Comment 3•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/42087/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42087/
Attachment #8734108 -
Flags: review?(felipc)
Assignee | ||
Comment 4•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/42077/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42077/
Attachment #8734109 -
Flags: review?(felipc)
Comment 5•8 years ago
|
||
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+
Assignee | ||
Comment 6•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=136d118d12f6
Assignee | ||
Comment 7•8 years ago
|
||
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/
Assignee | ||
Comment 8•8 years ago
|
||
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/
Assignee | ||
Comment 9•8 years ago
|
||
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/
Assignee | ||
Comment 10•8 years ago
|
||
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/
Assignee | ||
Comment 11•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/42339/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42339/
Attachment #8734532 -
Flags: review?(felipc)
Comment 12•8 years ago
|
||
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 13•8 years ago
|
||
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+
Updated•8 years ago
|
Attachment #8734108 -
Flags: review?(felipc) → review+
Comment 14•8 years ago
|
||
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
Updated•8 years ago
|
Attachment #8734532 -
Flags: review?(felipc) → review+
Comment 15•8 years ago
|
||
Comment on attachment 8734532 [details] MozReview Request: Bug 1254865 - Set disableglobalhistory on the thumbnail browser. r?felipe https://reviewboard.mozilla.org/r/42339/#review39137
Assignee | ||
Comment 16•8 years ago
|
||
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•8 years ago
|
||
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•8 years ago
|
||
hey, this seems to have caused a perf win in tpaint! https://treeherder.mozilla.org/perf.html#/alerts?id=653
Comment 19•8 years ago
|
||
bugherder |
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
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Assignee | ||
Comment 20•8 years ago
|
||
(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•8 years ago
|
||
(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/
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → mconley
Assignee | ||
Comment 22•8 years ago
|
||
bugnotes |
http://people.mozilla.org/~mconley2/bugnotes/bug-1254865.html
You need to log in
before you can comment on or make changes to this bug.
Description
•