Closed
Bug 1014185
Opened 10 years ago
Closed 8 years ago
Remove the about:customizing preloading hack
Categories
(Firefox :: Toolbars and Customization, defect)
Tracking
()
VERIFIED
FIXED
Firefox 47
Tracking | Status | |
---|---|---|
firefox47 | --- | verified |
People
(Reporter: mconley, Assigned: dao)
References
(Depends on 1 open bug)
Details
(Keywords: addon-compat, dev-doc-needed, user-doc-needed, Whiteboard: [MemShrink:P2])
Attachments
(1 file, 2 obsolete files)
43.34 KB,
patch
|
Details | Diff | Splinter Review |
As part of our effort to make the transition to customize smoother, we preload about:customizing[1] so that the cost of instantiating a docshell / browser doesn't jankify the transition. We preload about:customizing when the user opens the menu panel. That's a pretty bodacious hack, and means that we've got a docshell / browser just sitting in the background for the most part doing nothing. We should try to remove this hack to help conserve memory, and to generally simplify things. [1]: See bug 975552
Reporter | ||
Updated•10 years ago
|
OS: Windows 7 → All
Assignee | ||
Updated•10 years ago
|
Comment 1•10 years ago
|
||
Do you have any ideas on an approach that would allow this? Hard to see us finding a solution here.
Reporter | ||
Comment 2•10 years ago
|
||
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #1) > Do you have any ideas on an approach that would allow this? Hard to see us > finding a solution here. I know there were some ideas like making it possible for tabs to be browserless, or to share a browser, but that sounds pretty fragile, and might break some deep assumptions the rest of the codebase (and add-on ecosystem) make about tabbrowser. I think we need to find a way of not needing the browser in the tab to actually _load_ about:customizing, and still receive the notification that the user is attempting to browse there to initialize the transition. I think having the <browser> element be created is fine. It's the actual loading of the about:customizing document that is unnecessary.
Assignee | ||
Comment 3•10 years ago
|
||
I think the cleanest way would be to add an about:blank tab and stop the loading immediately...
Updated•10 years ago
|
Whiteboard: [MemShrink:P2]
Assignee | ||
Comment 5•8 years ago
|
||
Comment 6•8 years ago
|
||
Comment on attachment 8720853 [details] [diff] [review] patch Review of attachment 8720853 [details] [diff] [review]: ----------------------------------------------------------------- We may see some changes to CART due to this, how much of a regression are we willing to accept? ::: browser/components/build/nsModule.cpp @@ -106,5 @@ > { NS_ABOUT_MODULE_CONTRACTID_PREFIX "accounts", &kNS_BROWSER_ABOUT_REDIRECTOR_CID }, > #ifdef MOZ_SERVICES_HEALTHREPORT > { NS_ABOUT_MODULE_CONTRACTID_PREFIX "healthreport", &kNS_BROWSER_ABOUT_REDIRECTOR_CID }, > #endif > - { NS_ABOUT_MODULE_CONTRACTID_PREFIX "customizing", &kNS_BROWSER_ABOUT_REDIRECTOR_CID }, http://mxr.mozilla.org/mozilla-central/source/dom/plugins/test/mochitest/browser_bug1163570.js still has a reference to about:customizing
Attachment #8720853 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6) > We may see some changes to CART due to this, how much of a regression are we > willing to accept? Well, not much... I can't give you a specific number. I'll land this and see how it goes.
Attachment #8720853 -
Attachment is obsolete: true
Assignee | ||
Comment 9•8 years ago
|
||
So far it looks like there's a CART improvement with !e10s and a regression with e10s. I think forcing the blank tab to be non-remote would probably turn that regression around.
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c34fe673bb97
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Comment 11•8 years ago
|
||
backed this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=7403652&repo=fx-team
Status: RESOLVED → REOPENED
Flags: needinfo?(dao)
Resolution: FIXED → ---
Comment 12•8 years ago
|
||
Backout: https://hg.mozilla.org/mozilla-central/rev/69ec3dc408a2
Assignee | ||
Comment 13•8 years ago
|
||
Looks like that test is bogus and needs to be rewritten. I'll disable it and file a new bug.
Flags: needinfo?(dao)
Comment 14•8 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #13) > Looks like that test is bogus and needs to be rewritten. I'll disable it and > file a new bug. There were also some other failures that look related though, like: 73 INFO TEST-UNEXPECTED-FAIL | browser/components/customizableui/test/browser_876944_customize_mode_create_destroy.js | Uncaught exception - at :0 - Error: operation not possible on dead CPOW https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=26771bc857f8&selectedJob=7407922 and a few other CUI tests in that run with CPOW errors.
Assignee | ||
Comment 15•8 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #14) > Uncaught exception - at :0 Lovely.
Comment 16•8 years ago
|
||
As stated, this had some negative performance implications on the cart numbers before it was backed out: https://treeherder.allizom.org/perf.html#/alerts?id=198 More detailed breakdown on winxp: https://treeherder.allizom.org/perf.html#/comparesubtest?originalProject=fx-team&originalRevision=f9e4a56281d8&newProject=fx-team&newRevision=c34fe673bb97&originalSignature=e0de5c766545056e21f79d23c94bd212f2426249&newSignature=e0de5c766545056e21f79d23c94bd212f2426249
Assignee | ||
Comment 17•8 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #14) > (In reply to Dão Gottwald [:dao] from comment #13) > > Looks like that test is bogus and needs to be rewritten. I'll disable it and > > file a new bug. > > There were also some other failures that look related though, like: > > 73 INFO TEST-UNEXPECTED-FAIL | > browser/components/customizableui/test/ > browser_876944_customize_mode_create_destroy.js | Uncaught exception - at :0 > - Error: operation not possible on dead CPOW > > https://treeherder.mozilla.org/#/jobs?repo=fx- > team&revision=26771bc857f8&selectedJob=7407922 > > and a few other CUI tests in that run with CPOW errors. I suspect the patch in bug 1249587 might fix this.
Assignee | ||
Comment 18•8 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #17) > > and a few other CUI tests in that run with CPOW errors. > > I suspect the patch in bug 1249587 might fix this. It didn't. Maybe the problem is endCustomizing in browser/components/customizableui/test/head.js...
Assignee | ||
Comment 19•8 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #18) > Maybe the problem is endCustomizing in > browser/components/customizableui/test/head.js... Yeah, fixing endCustomizing seems to have helped. https://treeherder.mozilla.org/#/jobs?repo=try&revision=06859665a156&selectedJob=16990382
Attachment #8721055 -
Attachment is obsolete: true
Comment 21•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/48a7a7615da8
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 22•8 years ago
|
||
https://support.mozilla.org/en-US/kb/customize-firefox-controls-buttons-and-toolbars says "Tip: Another option to open Customize firefox tab is to type in Location bar about:customizing." We need to remove that note. Not that it was ever useful to end users in the first place...
Keywords: user-doc-needed
Comment 23•8 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #22) > https://support.mozilla.org/en-US/kb/customize-firefox-controls-buttons-and- > toolbars says "Tip: Another option to open Customize firefox tab is to type > in Location bar about:customizing." We need to remove that note. Not that it > was ever useful to end users in the first place... I submitted a revision. There are also several hits in the add-on MXR: https://mxr.mozilla.org/addons/search?string=about%3Acustomizing&find=.js , so I'm adding devdoc-needed (we should put this in the "Firefox 47 for developers" page) and addon-compat.
Keywords: addon-compat,
dev-doc-needed
Comment 24•8 years ago
|
||
I've reproduced this bug on Firefox Nightly 32.0a1(2014-05-21) on Ubuntu 14.04 LTS, 32-bit! The bug's fix is now verified on Latest Firefox Nightly 47.0a1! Build ID: 20160303030253 User Agent: Mozilla/5.0 (X11; Linux i686; rv:47.0) Gecko/20100101 Firefox/47.0
QA Whiteboard: [bugday-20160302]
Comment 25•8 years ago
|
||
I have reproduced this bug with Firefox Nightly 32.0a1 (Build ID: 20140521030200) on windows 8.1, 64-bit with the instructions from comment 0. Verified as fixed with Firefox Nightly 47.0a1 (Build ID: 20160307030208) Mozilla/5.0 (Windows NT 6.3; WOW64; rv:47.0) Gecko/20100101 Firefox/47.0 As this bug is also verified on Linux (Comment 24), I am marking this as verified! [bugday-20160302]
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•