Closed Bug 1517014 Opened 2 years ago Closed 2 years ago

AntiTrackingCommon::IsOnContentBlockingAllowList is slow


(Firefox :: Protections UI, enhancement, P1)




Firefox 66
Tracking Status
firefox66 --- fixed


(Reporter: ehsan, Assigned: ehsan)



(Whiteboard: [privacy65])


(4 files)

See this profile, for example:

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...
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.
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
Pushed by
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
Part 2: Avoid the overhead of parsing our origin string into a URI in AntiTrackingCommon::IsOnContentBlockingAllowList(); r=baku
Part 3: Avoid a hashtable lookup in AntiTrackingCommon::IsOnContentBlockingAllowList(); r=baku
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.