Make AntiTrackingCommon::MaybeIsFirstPartyStorageAccessGrantedFor() somewhat faster
Categories
(Firefox :: Protections UI, defect)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox67 | --- | fixed |
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(8 files)
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review |
This bug is filed for a grab bag of random optimizations made for improving the speed of this function in various ways.
|
Assignee | |
Comment 1•5 years ago
|
||
|
|
Assignee | |
Comment 2•5 years ago
|
||
Depends on D18643
|
|
Assignee | |
Comment 3•5 years ago
|
||
Depends on D18644
|
|
Assignee | |
Comment 4•5 years ago
|
||
Depends on D18645
|
|
Assignee | |
Comment 5•5 years ago
|
||
Depends on D18646
|
|
Assignee | |
Comment 6•5 years ago
|
||
Depends on D18647
|
|
Assignee | |
Comment 7•5 years ago
|
||
Depends on D18648
|
|
Assignee | |
Comment 8•5 years ago
|
||
Depends on D18649
|
||
Comment 9•5 years ago
|
||
Backed out for assertion mass failures on dom/base/ThirdPartyUtil.
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&selectedJob=226259418&revision=7a679beef53e6a356118373e7d831aa49f0e8e0b
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=226259418&repo=autoland&lineNumber=5434
Backout link: https://hg.mozilla.org/integration/autoland/rev/1e374e23c02f55d5bfa6ab445cea787c492aa2d6
|
||
Comment 10•5 years ago
|
||
Pushed by eakhgari@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bc4389577ba8 Part 1: Devirtualize the calls to ThirdPartyUtil in nsContentUtils::IsThirdPartyWindowOrChannel(); r=baku https://hg.mozilla.org/integration/autoland/rev/6afabd86b213 Part 2: Opportunistically avoid some allocations in ThirdPartyUtil.cpp; r=baku https://hg.mozilla.org/integration/autoland/rev/80d33523be6e Part 3: Cache the result of calling PR_StringToNetAddr() since it may be expensive; r=baku https://hg.mozilla.org/integration/autoland/rev/529fc4945c00 Part 4: Devirtualize the call to nsEffectiveTLDService::GetBaseDomain() from ThirdPartyUtil::GetBaseDomain(); r=baku https://hg.mozilla.org/integration/autoland/rev/461bf050580c Part 5: Avoid using SameCOMIdentity to compare windows inside ThirdPartyUtil::IsThirdPartyWindow(); r=baku https://hg.mozilla.org/integration/autoland/rev/40cdbb0f3e33 Part 6: Remove some needless AddRef/Release calls in the loop inside ThirdPartyUtil::IsThirdPartyWindow(); r=baku https://hg.mozilla.org/integration/autoland/rev/4a97d6a5440f Part 7: Devirtualize accesses to nsPermissionManager in the anti-tracking backend; r=baku
|
|
Assignee | |
Comment 11•5 years ago
|
||
This is the reason behind the assertion failure: https://phabricator.services.mozilla.com/D18643#479549
The fix is to remove the assertion.
|
|
||
Comment 12•5 years ago
|
||
Pushed by eakhgari@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/57699d28af86 Part 1: Devirtualize the calls to ThirdPartyUtil in nsContentUtils::IsThirdPartyWindowOrChannel(); r=baku https://hg.mozilla.org/integration/mozilla-inbound/rev/5767fa241fb1 Part 2: Opportunistically avoid some allocations in ThirdPartyUtil.cpp; r=baku https://hg.mozilla.org/integration/mozilla-inbound/rev/8628e3d716eb Part 3: Cache the result of calling PR_StringToNetAddr() since it may be expensive; r=baku https://hg.mozilla.org/integration/mozilla-inbound/rev/d740e9efc9e1 Part 4: Devirtualize the call to nsEffectiveTLDService::GetBaseDomain() from ThirdPartyUtil::GetBaseDomain(); r=baku https://hg.mozilla.org/integration/mozilla-inbound/rev/92d3ddffabbb Part 5: Avoid using SameCOMIdentity to compare windows inside ThirdPartyUtil::IsThirdPartyWindow(); r=baku https://hg.mozilla.org/integration/mozilla-inbound/rev/88aceed51d50 Part 6: Remove some needless AddRef/Release calls in the loop inside ThirdPartyUtil::IsThirdPartyWindow(); r=baku https://hg.mozilla.org/integration/mozilla-inbound/rev/75183a082bc1 Part 7: Devirtualize accesses to nsPermissionManager in the anti-tracking backend; r=baku https://hg.mozilla.org/integration/mozilla-inbound/rev/74b2c66643c9 Part 8: Avoid the overhead of calling NS_GetInnermostURI() in nsEffectiveTLDService; r=baku
|
||
Comment 13•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/57699d28af86
https://hg.mozilla.org/mozilla-central/rev/5767fa241fb1
https://hg.mozilla.org/mozilla-central/rev/8628e3d716eb
https://hg.mozilla.org/mozilla-central/rev/d740e9efc9e1
https://hg.mozilla.org/mozilla-central/rev/92d3ddffabbb
https://hg.mozilla.org/mozilla-central/rev/88aceed51d50
https://hg.mozilla.org/mozilla-central/rev/75183a082bc1
https://hg.mozilla.org/mozilla-central/rev/74b2c66643c9
|
||
Comment 14•5 years ago
|
||
How does this not leak:
https://hg.mozilla.org/integration/mozilla-inbound/rev/74b2c66643c9#l1.68
if we have a nested uri, but the call to GetInnermostURI somehow fails? The NS_RELEASE bit is only called if GetInnermostURI succeeds, which seems wrong (but unlikely?). Why wasn't the standard do_QueryInterface pattern used?
|
|
Assignee | |
Comment 15•5 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #14)
How does this not leak:
https://hg.mozilla.org/integration/mozilla-inbound/rev/74b2c66643c9#l1.68
if we have a nested uri, but the call to GetInnermostURI somehow fails? The NS_RELEASE bit is only called if GetInnermostURI succeeds, which seems wrong (but unlikely?). Why wasn't the standard do_QueryInterface pattern used?
It does and it should have been.
Description
•