Closed Bug 1492482 Opened 7 years ago Closed 7 years ago

Stop sending CPOWs from RemoteWebProgress.jsm

Categories

(Core :: DOM: Content Processes, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: mconley, Assigned: mconley)

References

Details

Attachments

(20 files, 2 obsolete files)

46 bytes, text/x-phabricator-request
mkaply
: review+
Details | Review
46 bytes, text/x-phabricator-request
Gijs
: review+
Details | Review
46 bytes, text/x-phabricator-request
florian
: review+
Details | Review
46 bytes, text/x-phabricator-request
johannh
: review+
Details | Review
46 bytes, text/x-phabricator-request
Gijs
: review+
Details | Review
46 bytes, text/x-phabricator-request
Gijs
: review+
Details | Review
46 bytes, text/x-phabricator-request
Felipe
: review+
Details | Review
46 bytes, text/x-phabricator-request
nhnt11
: review+
Details | Review
46 bytes, text/x-phabricator-request
mkaply
: review+
Details | Review
46 bytes, text/x-phabricator-request
mstange
: review+
Details | Review
46 bytes, text/x-phabricator-request
ursula
: review+
Details | Review
46 bytes, text/x-phabricator-request
ursula
: review+
Details | Review
46 bytes, text/x-phabricator-request
jdescottes
: review+
Details | Review
46 bytes, text/x-phabricator-request
jdescottes
: review+
Details | Review
46 bytes, text/x-phabricator-request
jdescottes
: review+
Details | Review
46 bytes, text/x-phabricator-request
Felipe
: review+
Details | Review
46 bytes, text/x-phabricator-request
Gijs
: review+
Details | Review
46 bytes, text/x-phabricator-request
Felipe
: review+
Details | Review
46 bytes, text/x-phabricator-request
baku
: review+
Details | Review
46 bytes, text/x-phabricator-request
Felipe
: review+
Details | Review
We send CPOWs as a way of populating things like remote-browser.xml's contentDocumentAsCPOW and contentWindowAsCPOW. Thanks to kmag, I've narrowed down the leak from bug 1472212 to the CPOWs that we're sending up. Instead of trying to work around it, I think enough is enough with the CPOWs - let's just try to get rid of them coming up from RemoteWebProgress.
Blocks: 1489762
Blocks: war-on-xbl
No longer blocks: war-on-xbl
(In reply to Mike Conley (:mconley) (:⚙️) from comment #2) > Making progress here: > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=9f3ef1acebec15d70dcedf9628e84b24d130e03f \o/
Comment on attachment 9011608 [details] Bug 1492482 - Get rid of tabbrowser.getBrowserIndexForDocument. r?mikedeboer Mike de Boer [:mikedeboer] has approved the revision.
Attachment #9011608 - Flags: review+
Attachment #9011608 - Attachment is obsolete: true
My patches, somehow, get rid of a circular reference in Protocol actors for DevTools, which means that they don't throw here: https://searchfox.org/mozilla-central/rev/ffe6eaf2f032e58ec3b0650a87df2c62ae4ca441/devtools/client/shared/redux/middleware/log.js#13 Historically, it was throwing, and we'd console.log the short thing, but now without the circular reference, we're logging the fully stringified objects which is causing the tests to time out. In bug 1473513, it looks like :yulia is fixing this for similar reasons, so setting up a dependency. yulia tells me that those patches are likely to land soon.
Depends on: 1473513
Comment on attachment 9012274 [details] Bug 1492482 - Remove CPOW usage from browser_tab_dragdrop2.js. r?Gijs :Gijs (out Thu 27 - Sun 30 / 9; he/him) has approved the revision.
Attachment #9012274 - Flags: review+
Comment on attachment 9012279 [details] Bug 1492482 - Remove CPOW usage from browser_urlbarHashChangeProxyState.js r?Gijs :Gijs (out Thu 27 - Sun 30 / 9; he/him) has approved the revision.
Attachment #9012279 - Flags: review+
Comment on attachment 9012281 [details] Bug 1492482 - Remove CPOW usage from browser_urlbarKeepStateAcrossTabSwitches.js. r?Gijs :Gijs (out Thu 27 - Sun 30 / 9; he/him) has approved the revision.
Attachment #9012281 - Flags: review+
This test appears to have been disabled since 2014, and about:home has changed a whole bunch since then. I also believe that searching from the new about:home is being tested elsewhere. Depends on D6956
Comment on attachment 9012277 [details] Bug 1492482 - Remove CPOW usage from browser_Bug 1492482.js r?johannh Johann Hofmann [:johannh] has approved the revision.
Attachment #9012277 - Flags: review+
Comment on attachment 9012302 [details] Bug 1492482 - Remove CPOW usage from browser_readerMode_with_anchor.js. r?Gijs :Gijs (out Thu 27 - Sun 30 / 9; he/him) has approved the revision.
Attachment #9012302 - Flags: review+
Comment on attachment 9012287 [details] Bug 1492482 - Remove CPOW usage from browser_onboarding_skip_tour.js. r?ursula Ursula Sarracini (:ursula) has approved the revision.
Attachment #9012287 - Flags: review+
This also "fixes" what appears to be some broken checks by switching them to todo()'s. I filed bug 1492885 to investigate these busted checks, and re-enable them. Depends on D6970
Comment on attachment 9012289 [details] Bug 1492482 - Remove CPOW usage from browser_onboarding_tourset.js. r?ursula Ursula Sarracini (:ursula) has approved the revision.
Attachment #9012289 - Flags: review+
Comment on attachment 9012285 [details] Bug 1492482 - Remove CPOW usage from browser_1119088.js. r?mstange Markus Stange [:mstange] has approved the revision.
Attachment #9012285 - Flags: review+
Comment on attachment 9012305 [details] Bug 1492482 - Stop using contentDocumentAsCPOW in browser_1119088.js. r?felipe :Felipe Gomes (needinfo me!) has approved the revision.
Attachment #9012305 - Flags: review+
Comment on attachment 9012303 [details] Bug 1492482 - Remove CPOW usage from browser_datetime_picker.js. r?felipe :Felipe Gomes (needinfo me!) has approved the revision.
Attachment #9012303 - Flags: review+
Comment on attachment 9012301 [details] Bug 1492482 - Remove CPOW usage from browser_aboutnewtab_process_selection.js. r?felipe :Felipe Gomes (needinfo me!) has approved the revision.
Attachment #9012301 - Flags: review+
Comment on attachment 9012282 [details] Bug 1492482 - Remove CPOW usage from browser_usercontext.js. r?felipe :Felipe Gomes (needinfo me!) has approved the revision.
Attachment #9012282 - Flags: review+
Comment on attachment 9012307 [details] Bug 1492482 - Stop sending CPOWs with WebProgressChild messages. r?felipe :Felipe Gomes (needinfo me!) has approved the revision.
Attachment #9012307 - Flags: review+
Comment on attachment 9012304 [details] Bug 1492482 - Remove CPOW usage from browser_saveImageURL.js. r?baku Andrea Marchesini [:baku] has approved the revision.
Attachment #9012304 - Flags: review+
Comment on attachment 9012275 [details] Bug 1492482 - Remove CPOW usage from browser_pageinfo_image_info.js r?florian Florian Quèze [:florian] has approved the revision.
Attachment #9012275 - Flags: review+
Comment on attachment 9012283 [details] Bug 1492482 - Remove CPOW usage from browser_aboutSearchReset.js. r?nhnt11 Nihanth Subramanya [:nhnt11] has approved the revision.
Attachment #9012283 - Flags: review+
Comment on attachment 9012284 [details] Bug 1492482 - Remove browser_abouthome_behavior.js test. r?mkaply Mike Kaply [:mkaply] has approved the revision.
Attachment #9012284 - Flags: review+
Comment on attachment 9012271 [details] Bug 1492482 - Remove CPOW usage from browser_keywordSearch_postData.js r?mkaply Mike Kaply [:mkaply] has approved the revision.
Attachment #9012271 - Flags: review+
Comment on attachment 9012300 [details] Bug 1492482 - Remove CPOWs from devtools/client/debugger/test/mochitest/head.js. r?jdescottes Julian Descottes [:jdescottes][:julian] has approved the revision.
Attachment #9012300 - Flags: review+
Comment on attachment 9012296 [details] Bug 1492482 - Remove CPOW usage from browser_dbg_event-listeners-02.js. r?jdescottes Julian Descottes [:jdescottes][:julian] has approved the revision.
Attachment #9012296 - Flags: review+
Comment on attachment 9012293 [details] Bug 1492482 - Remove CPOW usage from browser_dbg_event-listeners-01.js. r?jdescottes Julian Descottes [:jdescottes][:julian] has approved the revision.
Attachment #9012293 - Flags: review+
Attachment #9012307 - Attachment is obsolete: true
Attachment #9012307 - Attachment is obsolete: false
Attachment #9012305 - Attachment is obsolete: true
Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/56f92aa110f0 Remove CPOW usage from browser_keywordSearch_postData.js r=mkaply https://hg.mozilla.org/integration/autoland/rev/8bda47dfef15 Remove CPOW usage from browser_tab_dragdrop2.js. r=Gijs https://hg.mozilla.org/integration/autoland/rev/c25d430da5b7 Remove CPOW usage from browser_pageinfo_image_info.js r=florian https://hg.mozilla.org/integration/autoland/rev/9cedc276ac39 Remove CPOW usage from browser_Bug 1492482.js r=johannh https://hg.mozilla.org/integration/autoland/rev/159f859490af Remove CPOW usage from browser_urlbarHashChangeProxyState.js r=Gijs https://hg.mozilla.org/integration/autoland/rev/17c32940a63f Remove CPOW usage from browser_urlbarKeepStateAcrossTabSwitches.js. r=Gijs https://hg.mozilla.org/integration/autoland/rev/517aee4235da Remove CPOW usage from browser_usercontext.js. r=Felipe https://hg.mozilla.org/integration/autoland/rev/8e71dda1db82 Remove CPOW usage from browser_aboutSearchReset.js. r=nhnt11 https://hg.mozilla.org/integration/autoland/rev/62deabc00236 Remove browser_abouthome_behavior.js test. r=mkaply https://hg.mozilla.org/integration/autoland/rev/b34f1ec15697 Remove CPOW usage from browser_1119088.js. r=mstange https://hg.mozilla.org/integration/autoland/rev/6cbfcebf531f Remove CPOW usage from browser_onboarding_skip_tour.js. r=ursula https://hg.mozilla.org/integration/autoland/rev/0c2a8c43e38e Remove CPOW usage from browser_onboarding_tourset.js. r=ursula https://hg.mozilla.org/integration/autoland/rev/e36f11e68401 Remove CPOW usage from browser_dbg_event-listeners-01.js. r=jdescottes https://hg.mozilla.org/integration/autoland/rev/ac62c33e160e Remove CPOW usage from browser_dbg_event-listeners-02.js. r=jdescottes https://hg.mozilla.org/integration/autoland/rev/16d572f93a94 Remove CPOWs from devtools/client/debugger/test/mochitest/head.js. r=jdescottes https://hg.mozilla.org/integration/autoland/rev/fe01cfeef13a Remove CPOW usage from browser_aboutnewtab_process_selection.js. r=Felipe https://hg.mozilla.org/integration/autoland/rev/ae79eeefe39d Remove CPOW usage from browser_readerMode_with_anchor.js. r=Gijs https://hg.mozilla.org/integration/autoland/rev/62448675e862 Remove CPOW usage from browser_datetime_picker.js. r=Felipe https://hg.mozilla.org/integration/autoland/rev/7cfc774d8178 Remove CPOW usage from browser_saveImageURL.js. r=baku https://hg.mozilla.org/integration/autoland/rev/cfa50dc1f616 Stop sending CPOWs with WebProgressChild messages. r=Felipe
How many tests still have the uses-unsafe-cpows annotation after this?
Flags: needinfo?(mconley)
(In reply to Kris Maglione [:kmag] from comment #50) > How many tests still have the uses-unsafe-cpows annotation after this? Good question. I actually forgot to remove it from the ones I just updated here - filed bug 1495885 for that.
https://searchfox.org/mozilla-central/search?q=uses-unsafe-cpows+%3D+true&case=false&regexp=false&path= returns a list of 186 matches. My patch in bug 1495885 removes 54, so that leaves 132.
Flags: needinfo?(mconley)
(In reply to Mike Conley (:mconley) (:⚙️) from comment #52) > https://searchfox.org/mozilla-central/search?q=uses-unsafe- > cpows+%3D+true&case=false&regexp=false&path= returns a list of 186 matches. > My patch in bug 1495885 removes 54, so that leaves 132. :( Well, it's still an improvement. Thanks
Blocks: 1616881
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: