Bug 161441 – Should have a way for clients to specify which page loads should use content extensions

• Status: REOPENED
• Component: Page Loading

[Brian Weinstein]

Should have a way for clients to specify which page loads should use content extensions.

[Radar WebKit Bug Importer]

<rdar://problem/28098075>

[Brian Weinstein]

Created attachment 287519 [details]
[PATCH] Fix

[Alex Christensen]

Comment on attachment 287519 [details]
[PATCH] Fix

Do we really want this to be synchronous and in the InjectedBundle?  That means that each InjectedBundle would need to have enough information to immediately decide if we want to use content extensions for this URL, or it would need to send synchronous IPC.  We designed content extensions to not need either of these performance reducing things.

[Brian Weinstein]

(In reply to comment #3)
> Comment on attachment 287519 [details]
> [PATCH] Fix
> 
> Do we really want this to be synchronous and in the InjectedBundle?  That
> means that each InjectedBundle would need to have enough information to
> immediately decide if we want to use content extensions for this URL, or it
> would need to send synchronous IPC.  We designed content extensions to not
> need either of these performance reducing things.

In this case, the client is pushing the information to each injected bundle when it is updated, so the injected bundle always has the correct state of the world w.r.t. sites that shouldn’t use content blockers.

Another option is to push all of the information about which pages to disable content blockers on to WebKit/WebCore, but I’m not sure if that’s necessarily cleaner than what we are doing here.

[Alex Christensen]

I'm concerned about pushing long lists of URLs to each new WebProcess every time we make a new tab.

[Brian Weinstein]

(In reply to comment #5)
> I'm concerned about pushing long lists of URLs to each new WebProcess every
> time we make a new tab.

I agree with this concern in theory, but in practice:

- It wouldn't necessarily need a list of URLs, it could use high level domains
- I don't know how many domains users would store in practice.

Is there some code I can look at for how we would share one copy of this data across all web processes?

[Sam Weinig]

Perhaps this could be determined at decidePolicyForNavigationAction decision time in the UIProcess. It might be generally useful to be able to override functionality at that time, so adding a mechanism for it could be useful.

[Brian Weinstein]

(In reply to comment #7)
> Perhaps this could be determined at decidePolicyForNavigationAction decision
> time in the UIProcess. It might be generally useful to be able to override
> functionality at that time, so adding a mechanism for it could be useful.

That is an interesting idea! WKPageSetUserContentExtensionsEnabled seems to exist, but doesn't appear to be implemented.

[Brian Weinstein]

(In reply to comment #8)
> (In reply to comment #7)
> > Perhaps this could be determined at decidePolicyForNavigationAction decision
> > time in the UIProcess. It might be generally useful to be able to override
> > functionality at that time, so adding a mechanism for it could be useful.
> 
> That is an interesting idea! WKPageSetUserContentExtensionsEnabled seems to
> exist, but doesn't appear to be implemented.

But that might not be the way we would want to implement disabling content blockers for that load anyways, just something I noticed when clicking around.

[Brian Weinstein]

Another concern is whether or not the main resource load will have already started by the time the UI Process handles the decidePolicyForNavigationAction call.

[Alex Christensen]

decidePolicyForNavigationAction is initiated from didReceiveResponse in the NetworkProcess, which means it's too late; the main resource request has already been sent with content blockers applied

[Sam Weinig]

(In reply to comment #11)
> decidePolicyForNavigationAction is initiated from didReceiveResponse in the
> NetworkProcess, which means it's too late; the main resource request has
> already been sent with content blockers applied

We do the initial decidePolicyForNavigationAction before talking to the NetworkProcess.

[WebKit Commit Bot]

Comment on attachment 287519 [details]
[PATCH] Fix

Clearing flags on attachment: 287519

Committed r205407: <http://trac.webkit.org/changeset/205407>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Sam Weinig]

I don't this this is the correct direction for this feature. Please roll this out so we can continue our discussion of ways to get this functionality without additional bundle SPI.

[Ryan Haddad]

Reverted r205407 for reason:

Not the correct way to implement this functionality

Committed r205487: <http://trac.webkit.org/changeset/205487>