On Wed, Oct 19, 2016 at 10:19 AM, Chris Hutten-Czapski <chut...@mozilla.com>
wrote:
> Things that would help me help with this endeavour:
>
> 1: A bug to file patches against
>
I debated this, but given the number of tests involved that seemed like a
bunch of overhead. I've been filing bugs as I fix tests.
> 2: A method for detecting if our fix actually fixes the problem
>
I should have included this. What I do (and it isn't perfect but hopefully
not too bad, either) is to write my patch and then run |MOZ_CPOW_LOG=stacks
mach mochitest <test>.js|. That will give you stacks for all CPOWs, which
look like [1]:
INFO - #0 0x7f06ecc22140 i
chrome://mochitests/content/browser/browser/base/content/test/urlbar/browser_bug1025195_switchToTabHavingURI_aOpenParams.js:18
(0x7f06d04dc098 @ 21)
INFO - #1 0x7fff49c5cab0 b resource://gre/modules/Promise.jsm ->
resource://gre/modules/Promise-backend.js:388 (0x7f06ecfed808 @ 254)
INFO - #2 0x7f06ecc220a8 i
chrome://mochitests/content/browser/browser/base/content/test/urlbar/browser_bug1025195_switchToTabHavingURI_aOpenParams.js:17
(0x7f06d13f6f78 @ 252)
INFO - #3 0x7f06ecc22020 i self-hosted:1206 (0x7f06c747dc48 @ 24)
Note that if the stack includes RemoteAddonsParent.jsm, then the CPOW use
is almost certainly from a shim. My list explicitly filtered those out
since they shouldn't intermittently go orange because the shims use CPOWs
safely (they block the content process). That being said, I've been tending
to convert shim uses to BrowserTestUtils anyway. One extremely common shim
that I've seen used is |browser.addEventListener("load", ...)|. These shim
uses can usually be trivially converted to use
|BrowserTestUtils.browserLoaded(browser)|.
[1]
http://searchfox.org/mozilla-central/rev/d96317a351af8aa78ab9847e7feed964bbaac7d7/browser/base/content/test/urlbar/browser_bug1025195_switchToTabHavingURI_aOpenParams.js#18
> I presume a skeleton of what we're looking for is:
> 1) Use DXR/ls -r/whatever to find the test file in the tree
>
Yeah, test files in my experience are unique in the tree. I use |find .
name ...|.
2) On the line number(s) mentioned, replace the existing use of a CPOW with
> something better. This may involve writing things in terms of ContentTask
> (see documentation here[X]), or by simply finding a non-CPOW-using
> alternative (like using browser.webNavigation.sessionHistory instead of
> browser.sessionHistory)
>
We don't have a ton of documentation.
https://wiki.mozilla.org/Electrolysis/e10s_test_tips#Browser-chrome is
probably the best we have.
> 3) Run the test to make sure it still passes using ./mach test
> path/to/test.js
> 4) Write an informative commit message linking back to bug XXXX
> 5) Based on what kind of test it is, send it to try to make sure it isn't
> broken on other platforms
> 6) <INSERT VALIDATION OF CPOW REMOVAL STEP HERE>
> 7) Get the patch reviewed on bug XXXX
>
> Is this correct?
>
Yep.
--
Blake
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
