Closed Bug 1480319 Opened 7 years ago Closed 6 years ago

Minimize the amount of ContentLinkHandler code loaded by default

Categories

(Firefox :: General, enhancement, P1)

57 Branch
enhancement

Tracking

()

RESOLVED FIXED
Firefox 63
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.
(apparently I botched my commit and lost half of my changes the first time)
Triage: p1 as there are patches in active development
Priority: -- → P1
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
Attachment #8996920 - Flags: review?(dtownsend)
Comment on attachment 8996920 [details] Bug 1480319: Split favicon loading code out of ContentLinkHandler.jsm. https://reviewboard.mozilla.org/r/260910/#review269538
Attachment #8996920 - Flags: review?(dtownsend) → review+
Whiteboard: [overhead:18K] → [overhead:44K]
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
https://hg.mozilla.org/integration/mozilla-inbound/rev/186ad2d171c1f2348703b119ad5983515ead58a7 Bug 1480319: Follow-up: Re-enable favicon guessing during WebRequest tests. r=Mossop
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
Flags: needinfo?(kmaglione+bmo)
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Depends on: 1483910
Flags: needinfo?(kmaglione+bmo)

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 ).

(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.

(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.

See Also: → 1745680
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: