Closed Bug 1492482 Opened Last year Closed Last year

Stop sending CPOWs from RemoteWebProgress.jsm

Categories

(Core :: DOM: Content Processes, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: mconley, Assigned: mconley)

References

(Blocks 2 open bugs)

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
Duplicate of this bug: 1462552
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
You need to log in before you can comment on or make changes to this bug.