Minimize the amount of ContentLinkHandler code loaded by default
Categories
(Firefox :: General, enhancement, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: kmag, Assigned: kmag)
References
(Blocks 1 open bug)
Details
(Whiteboard: [overhead:44K])
Attachments
(1 file)
ContentLinkHandler contains a lot of code devoted to loading favicons, which is loaded in every content process, all the time. While it will generally always be needed for any content process that hosts a top-level HTTP document, it generally shouldn't be needed for many about: pages, and should never be needed out-of-process iframes. That code should all be split out into a separate module, and loaded only when we actually need to load a favicon.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•4 years ago
|
||
(apparently I botched my commit and lost half of my changes the first time)
Comment 5•4 years ago
|
||
mozreview-review |
Comment on attachment 8996920 [details] Bug 1480319: Split favicon loading code out of ContentLinkHandler.jsm. https://reviewboard.mozilla.org/r/260910/#review268744 I'm not seeing where the event listeners for the page are added now, is there a file missing here? Either way I want to see this rebased on top of the changes that just landed in bug 1478823
Comment hidden (mozreview-request) |
Comment 7•4 years ago
|
||
mozreview-review |
Comment on attachment 8996920 [details] Bug 1480319: Split favicon loading code out of ContentLinkHandler.jsm. https://reviewboard.mozilla.org/r/260910/#review269538
Assignee | ||
Comment 8•4 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa14dcca91fb46a6e44a68d2b0370dd0eff8d400 Bug 1480319: Split favicon loading code out of ContentLinkHandler.jsm. r=Mossop
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 9•4 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/30a0f8cede1eb5d4bde81fd3d05006d2d2062433 Bug 1480319: Follow-up: Fix URL in browser_startup_content test. r=bustage
Assignee | ||
Comment 10•4 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0161b3bbc9d21cb915ffb3fe6b0aead720141c0b Bug 1480319: Follow-up: Don't try to guess favicons during WebRequest test. r=bustage CLOSED TREE
Assignee | ||
Comment 11•4 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/186ad2d171c1f2348703b119ad5983515ead58a7 Bug 1480319: Follow-up: Re-enable favicon guessing during WebRequest tests. r=Mossop
Comment 12•4 years ago
|
||
Backed out for mochitest failures on test_ext_webrequest_filter.html. backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/e19fc7991514a647468e3b80c798db58b06cf15b push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=186ad2d171c1f2348703b119ad5983515ead58a7&selectedJob=194194128 failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=194202843&repo=mozilla-inbound&lineNumber=5529 00:16:43 INFO - TEST-PASS | toolkit/components/extensions/test/mochitest/test-oop-extensions/test_ext_webrequest_filter.html | correct tabId - Expected: 46, Actual: 46 00:16:43 INFO - Buffered messages finished 00:16:43 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/mochitest/test-oop-extensions/test_ext_webrequest_filter.html | Test timed out. 00:16:43 INFO - reportError@SimpleTest/TestRunner.js:121:7 00:16:43 INFO - TestRunner._checkForHangs@SimpleTest/TestRunner.js:142:7 00:16:43 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:43 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:43 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:43 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:43 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:43 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:43 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:43 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:43 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:43 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:43 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:43 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:43 INFO - TestRunner.runTests/<@SimpleTest/TestRunner.js:381:9 00:16:43 INFO - promise callback*TestRunner.runTests@SimpleTest/TestRunner.js:368:50 00:16:43 INFO - RunSet.runtests@SimpleTest/setup.js:194:3 00:16:43 INFO - RunSet.runall@SimpleTest/setup.js:173:5 00:16:43 INFO - hookupTests@SimpleTest/setup.js:266:5 00:16:43 INFO - parseTestManifest@http://mochi.test:8888/manifestLibrary.js:36:5 00:16:43 INFO - getTestManifest/req.onload@http://mochi.test:8888/manifestLibrary.js:49:11 00:16:43 INFO - EventHandlerNonNull*getTestManifest@http://mochi.test:8888/manifestLibrary.js:45:3 00:16:43 INFO - hookup@SimpleTest/setup.js:246:5 00:16:43 INFO - EventHandlerNonNull*@http://mochi.test:8888/tests?autorun=1&closeWhenDone=1&consoleLevel=INFO&manifestFile=tests.json&dumpOutputDirectory=c%3A%5Cusers%5Ctask_1534375853%5Cappdata%5Clocal%5Ctemp&cleanupCrashes=true:11:1 00:16:44 INFO - Not taking screenshot here: see the one that was previously logged 00:16:44 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/mochitest/test-oop-extensions/test_ext_webrequest_filter.html | Extension left running at test shutdown 00:16:44 INFO - ExtensionTestUtils.loadExtension/<@SimpleTest/ExtensionTestUtils.js:109:7 00:16:44 INFO - executeCleanupFunction@SimpleTest/SimpleTest.js:1227:19 00:16:44 INFO - SimpleTest.finish@SimpleTest/SimpleTest.js:1240:5 00:16:44 INFO - killTest@SimpleTest/TestRunner.js:130:7 00:16:44 INFO - delayedKillTest@SimpleTest/TestRunner.js:157:47 00:16:44 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:157:7 00:16:44 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:44 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:44 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:44 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:44 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:44 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:44 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:44 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:44 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:44 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:44 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:44 INFO - setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5 00:16:44 INFO - TestRunner.runTests/<@SimpleTest/TestRunner.js:381:9 00:16:44 INFO - promise callback*TestRunner.runTests@SimpleTest/TestRunner.js:368:50 00:16:44 INFO - RunSet.runtests@SimpleTest/setup.js:194:3 00:16:44 INFO - RunSet.runall@SimpleTest/setup.js:173:5 00:16:44 INFO - hookupTests@SimpleTest/setup.js:266:5
Assignee | ||
Comment 13•4 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5746fe67e2d252035ca8257bd36d6b40d80fde39 Bug 1480319: Split favicon loading code out of ContentLinkHandler.jsm. r=Mossop
Comment 14•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5746fe67e2d2
Assignee | ||
Updated•4 years ago
|
Comment 15•2 years ago
|
||
This is an extremely minor issue report, but I've noticed that Firefox 74.0 is -- unreliably, perhaps due to a race condition -- sometimes making requests for a favicon.ico at a base domain path (GET /favicon.ico) despite the page specifying alternate, specific rel="icon" paths.
The investigation I'd performed so far is visible at https://github.com/grocy/grocy/pull/692 and I tracked this back via the LinkHandlerChild call (at tag FIREFOX_NIGHTLY_74_END) to the commit associated with this bug ( https://hg.mozilla.org/mozilla-central/annotate/f53ecf8eee2a0d2a7705d7ea3ec8682028e922a0/browser/actors/LinkHandlerChild.jsm#l72 ).
Comment 16•2 years ago
|
||
(In reply to James Addison from comment #15)
This is an extremely minor issue report, but I've noticed that Firefox 74.0 is -- unreliably, perhaps due to a race condition -- sometimes making requests for a favicon.ico at a base domain path (GET /favicon.ico) despite the page specifying alternate, specific rel="icon" paths.
The investigation I'd performed so far is visible at https://github.com/grocy/grocy/pull/692 and I tracked this back via the LinkHandlerChild call (at tag FIREFOX_NIGHTLY_74_END) to the commit associated with this bug ( https://hg.mozilla.org/mozilla-central/annotate/f53ecf8eee2a0d2a7705d7ea3ec8682028e922a0/browser/actors/LinkHandlerChild.jsm#l72 ).
Please file a new bug for this issue.
Comment 17•2 years ago
|
||
(In reply to Dave Townsend [:mossop] (he/him) from comment #16)
Please file a new bug for this issue.
Thanks - I've identified https://bugzilla.mozilla.org/show_bug.cgi?id=1186324 as an existing bug for the same issue.
Description
•