AntiTrackingCommon::IsOnContentBlockingAllowList is slow

RESOLVED FIXED in Firefox 66

Status

()

enhancement
P1
normal
RESOLVED FIXED
4 months ago
3 months ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

(Blocks 1 bug)

unspecified
Firefox 66
Points:
---

Firefox Tracking Flags

(firefox66 fixed)

Details

(Whiteboard: [privacy65])

Attachments

(4 attachments)

(Assignee)

Description

4 months ago
See this profile, for example:

https://perfht.ml/2R05y0J

There is string parsing, URL parsing, and a hashtable lookup going on there.  Note that we call this function from nsContentUtils::LoadImage as well as nsContentUtils::IsImageInCache.  So much sadness...
(Assignee)

Comment 1

4 months ago
For consumers which have an origin string, currently they need to parse it into
a URI before they can call testPermission().  Internally, in the common case this
nsIURI* argument will be immediately converted back into the same origin string
in PermissionKey::CreateFromURI().  This means that the cost of parsing the
original origin string will effectively end up being wasted in the common case.

This patch adds an API that allows the consumer to test a permission using the
origin string directly, and only parse it into an nsIURI when necessary, thereby
avoiding this overhead.
(Assignee)

Comment 4

4 months ago
The only implementation of nsIURI which has a GetHostPort() method which can
succeed is nsStandardURL, which implements nsIURL.  Other implementations
either do not implement nsIURL (so in the old version, they would bail out
early) or they do, but their GetHostPort() method returns an error code
unconditionally (so in the old version, they would bail out later).  Hence,
this patch doesn't change the semantics of the code.

Depends on D15545
Priority: -- → P1
Whiteboard: [privacy65]
Assignee: nobody → ehsan
Status: NEW → ASSIGNED

Comment 5

3 months ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d316c3634857
Part 1: Add nsIPermissionManager.testPermissionOriginNoSuffix(), an API for testing permissions using an origin string without the overhead of parsing it into a URI; r=nika
https://hg.mozilla.org/integration/autoland/rev/da74b409b3da
Part 2: Avoid the overhead of parsing our origin string into a URI in AntiTrackingCommon::IsOnContentBlockingAllowList(); r=baku
https://hg.mozilla.org/integration/autoland/rev/28a3d1a99256
Part 3: Avoid a hashtable lookup in AntiTrackingCommon::IsOnContentBlockingAllowList(); r=baku
https://hg.mozilla.org/integration/autoland/rev/14d8f03b496c
Part 4: Remove a useless QueryInterface from AntiTrackingCommon::IsOnContentBlockingAllowList(); r=baku
You need to log in before you can comment on or make changes to this bug.