Add support for shimming resources blocked by Tracking Protection
Categories
(Web Compatibility :: Interventions, enhancement, P1)
Tracking
(firefox81 fixed)
Tracking | Status | |
---|---|---|
firefox81 | --- | fixed |
People
(Reporter: miketaylr, Assigned: twisniewski)
References
(Depends on 1 open bug, Blocks 3 open bugs)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
In order to minimize user-facing site bustage when Strict Tracking Protection is enabled, we would like to have the ability to "shim" certain resources or popular SDKs (for logins, etc).
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 1•4 years ago
|
||
Comment 3•4 years ago
|
||
Backed out for perma failures.
Log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=310283998&repo=autoland&lineNumber=1564
Backout: https://hg.mozilla.org/integration/autoland/rev/debc5d3ec218285df7e724af197646a7935cafb2
Assignee | ||
Comment 4•4 years ago
|
||
robwu, jwatt, I have literally no idea what is going on with the permafailure shown in comment 3 here. Any ideas? I was hoping to land this before I went on PTO (because of the soft-freeze while I'm away), but I would rather not just blindly disable the shims this patch adds when e10s is off, without knowing what the error actually is. Why would this test get even worse with my patch, and does it indicate a deeper problem we might need to address? (I see the test was already disabled on Windows recently, so I have to wonder). Or if it's somehow deemed alright to land my patch and just disable that test for now, would either of you mind doing that? (I'll be away from my office in a few hours).
Comment 5•4 years ago
|
||
The stack of the crash from comment 3 points to the Localization destructor, and I don't see an obvious relation between your patch and that. I don't even see retriggers, so I don't know why it is classified as a permafail. I clicked on "Retrigger job" to retrigger the job, and the test passes, so it is clearly not a permafail: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedTaskRun=Ft6WVGk-S5qP4d6j0Hk_qA.0&revision=b0b40eebd9abf2748f85d7c700a2ced36035b14a&searchStr=os%2Cx%2C10.14%2Cdebug%2Cmochitests%2Cwithout%2Ce10s%2Ctest-macosx1014-64%2Fdebug-mochitest-chrome-1proc%2Cc2
The test is already disabled on non-e10s Linux and soon Windows (bug 1583315)., so I think that the test is just unreliable. I suggest to try to reland it.
For posterity, the stack of the crash is:
INFO - GECKO(9300) | Assertion failure: !mNext (Must be unlinked), at /builds/worker/checkouts/gecko/js/xpconnect/src/xpcprivate.h:266
INFO - GECKO(9300) | #01: nsXPCWrappedJS::~nsXPCWrappedJS() [js/xpconnect/src/XPCWrappedJS.cpp:430]
INFO - GECKO(9300) | #02: nsXPCWrappedJS::Release() [js/xpconnect/src/XPCWrappedJS.cpp:271]
INFO - GECKO(9300) | #03: nsXPCWrappedJS::Release() [js/xpconnect/src/XPCWrappedJS.cpp:286]
INFO - GECKO(9300) | #04: mozilla::intl::Localization::~Localization() [intl/l10n/Localization.cpp:146]
INFO - GECKO(9300) | #05: mozilla::intl::Localization::~Localization() [intl/l10n/Localization.cpp:136]
INFO - GECKO(9300) | #06: SuspectAfterShutdown(void*, nsCycleCollectionParticipant*, nsCycleCollectingAutoRefCnt*, bool*) [xpcom/base/nsCycleCollector.cpp:3741]
INFO - GECKO(9300) | #07: mozilla::intl::Localization::Release() [intl/l10n/Localization.cpp:44]
INFO - GECKO(9300) | #08: mozilla::SegmentedVector<nsCOMPtr<nsISupports>, 4096ul, mozilla::MallocAllocPolicy>::SegmentImpl<509ul>::~SegmentImpl() [mfbt/SegmentedVector.h:76]
INFO - GECKO(9300) | #09: mozilla::SegmentedVector<nsCOMPtr<nsISupports>, 4096ul, mozilla::MallocAllocPolicy>::PopLastN(unsigned int) [mfbt/SegmentedVector.h:247]
INFO - GECKO(9300) | #10: mozilla::dom::DeferredFinalizerImpl<nsISupports>::DeferredFinalize(unsigned int, void*) [dom/bindings/BindingUtils.h:2731]
INFO - GECKO(9300) | #11: mozilla::IncrementalFinalizeRunnable::ReleaseNow(bool) [xpcom/base/CycleCollectedJSRuntime.cpp:1553]
INFO - GECKO(9300) | #12: mozilla::CycleCollectedJSRuntime::FinalizeDeferredThings(mozilla::CycleCollectedJSContext::DeferredFinalizeType) [xpcom/base/CycleCollectedJSRuntime.cpp:1627]
INFO - GECKO(9300) | #13: mozilla::CycleCollectedJSRuntime::OnGC(JSContext*, JSGCStatus, JS::GCReason) [xpcom/base/CycleCollectedJSRuntime.cpp:1706]
INFO - GECKO(9300) | #14: js::gc::GCRuntime::maybeCallGCCallback(JSGCStatus, JS::GCReason) [js/src/gc/GC.cpp:7033]
INFO - GECKO(9300) | #15: js::gc::GCRuntime::gcCycle(bool, js::SliceBudget, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason) [js/src/gc/GC.cpp:7124]
INFO - GECKO(9300) | #16: js::gc::GCRuntime::collect(bool, js::SliceBudget, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason) [js/src/gc/GC.cpp:7347]
INFO - GECKO(9300) | #17: js::gc::GCRuntime::gc(JSGCInvocationKind, JS::GCReason) [js/src/gc/GC.cpp:7423]
INFO - GECKO(9300) | #18: JSRuntime::destroyRuntime() [js/src/vm/Runtime.cpp:290]
INFO - GECKO(9300) | #19: js::DestroyContext(JSContext*) [js/src/vm/JSContext.cpp:214]
INFO - GECKO(9300) | #20: mozilla::CycleCollectedJSContext::~CycleCollectedJSContext() [xpcom/base/CycleCollectedJSContext.cpp:109]
INFO - GECKO(9300) | #21: XPCJSContext::~XPCJSContext() [js/xpconnect/src/XPCJSContext.cpp:1063]
INFO - GECKO(9300) | #22: XPCJSContext::~XPCJSContext() [js/xpconnect/src/XPCJSContext.cpp:1027]
INFO - GECKO(9300) | #23: nsXPConnect::~nsXPConnect() [js/xpconnect/src/nsXPConnect.cpp:128]
INFO - GECKO(9300) | #24: nsXPConnect::~nsXPConnect() [js/xpconnect/src/nsXPConnect.cpp:97]
INFO - GECKO(9300) | #25: nsXPConnect::ReleaseXPConnectSingleton() [js/xpconnect/src/nsXPConnect.cpp:164]
INFO - GECKO(9300) | #26: nsComponentManagerImpl::Shutdown() [xpcom/components/nsComponentManager.cpp:933]
INFO - GECKO(9300) | #27: mozilla::ShutdownXPCOM(nsIServiceManager*) [xpcom/build/XPCOMInit.cpp:737]
INFO - GECKO(9300) | #28: ScopedXPCOMStartup::~ScopedXPCOMStartup() [toolkit/xre/nsAppRunner.cpp:1286]
INFO - GECKO(9300) | #29: XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) [toolkit/xre/nsAppRunner.cpp:4926]
INFO - GECKO(9300) | #30: XRE_main(int, char**, mozilla::BootstrapConfig const&) [toolkit/xre/nsAppRunner.cpp:4963]
INFO - GECKO(9300) | #31: main [/Users/cltbld/tasks/task_1595082452/build/application/Firefox NightlyDebug.app/Contents/MacOS/firefox + 0x1624]
INFO - Not taking screenshot here: see the one that was previously logged
INFO - TEST-UNEXPECTED-FAIL | browser/components/shell/test/test_headless_screenshot.html | Firefox process should exit with code 0 - got 1, expected +0
Assignee | ||
Comment 6•4 years ago
|
||
Ok, I've queued it up for landing, hopefully it sticks this time. (Sorry otherwise!)
Comment 8•4 years ago
|
||
Backed out for assertion failures on xpcprivate.h
backout: https://hg.mozilla.org/integration/autoland/rev/015515bcba1f358432cad988600672d027fd125d
failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=310325896&repo=autoland&lineNumber=1390
[task 2020-07-19T23:55:29.208Z] 23:55:29 INFO - GECKO(1159) | Assertion failure: !mNext (Must be unlinked), at /builds/worker/checkouts/gecko/js/xpconnect/src/xpcprivate.h:266
[task 2020-07-19T23:55:29.224Z] 23:55:29 INFO - Initializing stack-fixing for the first stack frame, this may take a while...
[task 2020-07-19T23:55:49.707Z] 23:55:49 INFO - GECKO(1159) | #01: nsXPCWrappedJS::~nsXPCWrappedJS() [js/xpconnect/src/XPCWrappedJS.cpp:430]
[task 2020-07-19T23:55:49.708Z] 23:55:49 INFO - GECKO(1159) | #02: nsXPCWrappedJS::Release() [js/xpconnect/src/XPCWrappedJS.cpp:271]
[task 2020-07-19T23:55:49.708Z] 23:55:49 INFO - GECKO(1159) | #03: nsXPCWrappedJS::Release() [js/xpconnect/src/XPCWrappedJS.cpp:286]
[task 2020-07-19T23:55:49.709Z] 23:55:49 INFO - GECKO(1159) | #04: mozilla::intl::Localization::~Localization() [intl/l10n/Localization.cpp:146]
[task 2020-07-19T23:55:49.710Z] 23:55:49 INFO - GECKO(1159) | #05: mozilla::intl::Localization::~Localization() [intl/l10n/Localization.cpp:136]
[task 2020-07-19T23:55:49.710Z] 23:55:49 INFO - GECKO(1159) | #06: SuspectAfterShutdown(void*, nsCycleCollectionParticipant*, nsCycleCollectingAutoRefCnt*, bool*) [xpcom/base/nsCycleCollector.cpp:3741]
[task 2020-07-19T23:55:49.710Z] 23:55:49 INFO - GECKO(1159) | #07: mozilla::intl::Localization::Release() [intl/l10n/Localization.cpp:44]
[task 2020-07-19T23:55:49.711Z] 23:55:49 INFO - GECKO(1159) | #08: mozilla::SegmentedVector<nsCOMPtr<nsISupports>, 4096ul, mozilla::MallocAllocPolicy>::SegmentImpl<509ul>::~SegmentImpl() [mfbt/SegmentedVector.h:76]
[task 2020-07-19T23:55:49.711Z] 23:55:49 INFO - GECKO(1159) | #09: mozilla::SegmentedVector<nsCOMPtr<nsISupports>, 4096ul, mozilla::MallocAllocPolicy>::PopLastN(unsigned int) [mfbt/SegmentedVector.h:247]
[task 2020-07-19T23:55:49.715Z] 23:55:49 INFO - GECKO(1159) | #10: mozilla::dom::DeferredFinalizerImpl<nsISupports>::DeferredFinalize(unsigned int, void*) [dom/bindings/BindingUtils.h:2731]
[task 2020-07-19T23:55:49.716Z] 23:55:49 INFO - GECKO(1159) | #11: mozilla::IncrementalFinalizeRunnable::ReleaseNow(bool) [xpcom/base/CycleCollectedJSRuntime.cpp:1553]
[task 2020-07-19T23:55:49.716Z] 23:55:49 INFO - GECKO(1159) | #12: mozilla::CycleCollectedJSRuntime::FinalizeDeferredThings(mozilla::CycleCollectedJSContext::DeferredFinalizeType) [xpcom/base/CycleCollectedJSRuntime.cpp:1627]
[task 2020-07-19T23:55:49.716Z] 23:55:49 INFO - GECKO(1159) | #13: mozilla::CycleCollectedJSRuntime::OnGC(JSContext*, JSGCStatus, JS::GCReason) [xpcom/base/CycleCollectedJSRuntime.cpp:1706]
[task 2020-07-19T23:55:49.716Z] 23:55:49 INFO - GECKO(1159) | #14: js::gc::GCRuntime::maybeCallGCCallback(JSGCStatus, JS::GCReason) [js/src/gc/GC.cpp:7033]
[task 2020-07-19T23:55:49.717Z] 23:55:49 INFO - GECKO(1159) | #15: js::gc::GCRuntime::gcCycle(bool, js::SliceBudget, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason) [js/src/gc/GC.cpp:7124]
[task 2020-07-19T23:55:49.717Z] 23:55:49 INFO - GECKO(1159) | #16: js::gc::GCRuntime::collect(bool, js::SliceBudget, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason) [js/src/gc/GC.cpp:7347]
[task 2020-07-19T23:55:49.717Z] 23:55:49 INFO - GECKO(1159) | #17: js::gc::GCRuntime::gc(JSGCInvocationKind, JS::GCReason) [js/src/gc/GC.cpp:7423]
[task 2020-07-19T23:55:49.717Z] 23:55:49 INFO - GECKO(1159) | #18: JSRuntime::destroyRuntime() [js/src/vm/Runtime.cpp:290]
[task 2020-07-19T23:55:49.718Z] 23:55:49 INFO - GECKO(1159) | #19: js::DestroyContext(JSContext*) [js/src/vm/JSContext.cpp:214]
[task 2020-07-19T23:55:49.718Z] 23:55:49 INFO - GECKO(1159) | #20: mozilla::CycleCollectedJSContext::~CycleCollectedJSContext() [xpcom/base/CycleCollectedJSContext.cpp:109]
[task 2020-07-19T23:55:49.718Z] 23:55:49 INFO - GECKO(1159) | #21: XPCJSContext::~XPCJSContext() [js/xpconnect/src/XPCJSContext.cpp:1063]
[task 2020-07-19T23:55:49.718Z] 23:55:49 INFO - GECKO(1159) | #22: XPCJSContext::~XPCJSContext() [js/xpconnect/src/XPCJSContext.cpp:1027]
[task 2020-07-19T23:55:49.718Z] 23:55:49 INFO - GECKO(1159) | #23: nsXPConnect::~nsXPConnect() [js/xpconnect/src/nsXPConnect.cpp:128]
[task 2020-07-19T23:55:49.718Z] 23:55:49 INFO - GECKO(1159) | #24: nsXPConnect::~nsXPConnect() [js/xpconnect/src/nsXPConnect.cpp:97]
[task 2020-07-19T23:55:49.719Z] 23:55:49 INFO - GECKO(1159) | #25: nsXPConnect::ReleaseXPConnectSingleton() [js/xpconnect/src/nsXPConnect.cpp:164]
[task 2020-07-19T23:55:49.719Z] 23:55:49 INFO - GECKO(1159) | #26: nsComponentManagerImpl::Shutdown() [xpcom/components/nsComponentManager.cpp:933]
[task 2020-07-19T23:55:49.719Z] 23:55:49 INFO - GECKO(1159) | #27: mozilla::ShutdownXPCOM(nsIServiceManager*) [xpcom/build/XPCOMInit.cpp:737]
[task 2020-07-19T23:55:49.719Z] 23:55:49 INFO - GECKO(1159) | #28: ScopedXPCOMStartup::~ScopedXPCOMStartup() [toolkit/xre/nsAppRunner.cpp:1286]
[task 2020-07-19T23:55:49.721Z] 23:55:49 INFO - GECKO(1159) | #29: XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) [toolkit/xre/nsAppRunner.cpp:4926]
[task 2020-07-19T23:55:49.722Z] 23:55:49 INFO - GECKO(1159) | #30: XRE_main(int, char**, mozilla::BootstrapConfig const&) [toolkit/xre/nsAppRunner.cpp:4963]
[task 2020-07-19T23:55:49.722Z] 23:55:49 INFO - fix-stacks error: failed to read breakpad symbols dir/Users/cltbld/tasks/task_1595202714/build/symbols/firefox
for/Users/cltbld/tasks/task_1595202714/build/application/Firefox NightlyDebug.app/Contents/MacOS/firefox
[task 2020-07-19T23:55:49.722Z] 23:55:49 INFO - fix-stacks note: this is expected and harmless for system libraries on debug automation runs
[task 2020-07-19T23:55:49.722Z] 23:55:49 INFO - GECKO(1159) | #31: main [/Users/cltbld/tasks/task_1595202714/build/application/Firefox NightlyDebug.app/Contents/MacOS/firefox + 0x1624]
[task 2020-07-19T23:55:49.722Z] 23:55:49 INFO - TEST-INFO | started process screencapture
[task 2020-07-19T23:55:49.862Z] 23:55:49 INFO - TEST-INFO | screencapture: exit 0
Comment 9•4 years ago
|
||
Do you know why the crash in ~Localization
is happening? See comment 5 for the formatted crash.
The crash happened at the first attempt to land, did not happen when I retriggered, and happened again on the second attempt to land the patch.
Comment 10•4 years ago
|
||
I don't see a reason for that, but it's originated in mozilla::DropJSObjects(this);
which we recently added in bug 1631593. it may be that the recent changes to shutdown are flipping some shutdown race.
Olli was reviewing the patch that added it, so maybe he'll have an idea how DropJSObjects
gets called so late (should it just do nothing at this stage of the shutdown?)
Comment 11•4 years ago
|
||
Why would it be originated to mozilla::DropJSObjects(this);?
The crash happens when we implicitly release Localization::mLocalization at the end of the destructor.
Comment 12•4 years ago
|
||
There was a similar crash in Thunderbird, in bug 1629495, which got "fixed" by disabling some tests.
Both browser/components/shell/test/test_headless_screenshot.html
and the failing Thunderbird tests ran without e10s.
Would you be fine with disabling the test, relanding the patch (that has nothing to do with Localization
) and debugging this in a follow-up bug?
Or do you think that we can keep it disabled because non-e10s is not really supported any more?
Comment 13•4 years ago
|
||
Ah, so [task 2020-07-19T23:55:49.709Z] 23:55:49 INFO - GECKO(1159) | #04: mozilla::intl::Localization::~Localization() [intl/l10n/Localization.cpp:146]
is not https://searchfox.org/mozilla-central/source/intl/l10n/Localization.cpp#145
It's the implicit release of mLocalization
which happens after.
Would it help to release it in Destroy
which would then be covered by NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN
?
Comment 14•4 years ago
|
||
I don't think that will make a difference, as you're already running NS_IMPL_CYCLE_COLLECTION_UNLINK(mLocalization)
there, which will null out mLocalization
if the type is ever unlinked: https://searchfox.org/mozilla-central/rev/3b6958c26049c1e27b2790a43154caaba9f6dd4a/intl/l10n/Localization.cpp#24
Comment 15•4 years ago
|
||
A possible patch for the crash (just based on code inspection by Nika and me)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bf8158b6fbdd960e355dd159aa50d5fa9e6835cc
oops, I had something unrelated on that tree too. Oh well, that shouldn't affect to shutdown.
Comment 16•4 years ago
|
||
Rob, if you know how to reproduce the crash, even if just on tryserver, could you try this patch https://hg.mozilla.org/try/rev/8abbccf0d0572087f0daf1d58eb06a695d03459b
Comment 17•4 years ago
|
||
Two attempts to landing it failed, but a retrigger succeeded, so I don't think that I am able to consistently reproduce.
If your patch doesn't cause issues (i.e. your try push with the patch in isolation was green), then I suggest to send your patch to Phabricator, let the patch stack here depend on your patch, and then land all patches together. If your patch fixes the issue, the patchset will stick. If it doesn't fix the issue, then this patch may eventually be backed out.
Comment 18•4 years ago
|
||
Does any of that need to happen before the merge? I'd rather land the patch after the merge.
Comment 19•4 years ago
|
||
I think that :twisniewski would prefer to land this before the branch, but since it's a huge patch and we're investigating an unknown issue, I would recommend to wait after the branch instead. Then we have four more weeks to comfortably investigate issues in case things go south.
Assignee | ||
Comment 20•4 years ago
|
||
Agreed. I would have preferred to land this before the branch, but if there is risk involved with this fix, then let's wait.
Assignee | ||
Comment 21•4 years ago
|
||
Given that the patch in bug 1655778 has successfully landed, I'm going to try landing this patch again to confirm whether it fixes the test-failure (with the trivial rebasing required since I tried landing last time).
Comment 22•4 years ago
|
||
Comment 23•4 years ago
|
||
bugherder |
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Comment 24•4 years ago
|
||
Updated•3 years ago
|
Description
•