Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29740555: Issue 6469 - Inject API wrappers only if there's a filter match (Closed)

Created:
April 2, 2018, 9:47 a.m. by Manish Jethani
Modified:
April 27, 2018, 2:02 p.m.
Reviewers:
Sebastian Noack
CC:
kzar
Base URL:
https://hg.adblockplus.org/adblockpluschrome/
Visibility:
Public.

Description

Most URLs don't have a matching WebRTC filter, and that's the only reason we inject the API wrappers at all. We can remove the wrapper code from the default content script and inject it only if there's a filter match. webNavigation.onCommitted is dispatched at about the same time the default content script is loaded. There may be a delay, but it's probably not enough for other scripts in the document to finished downloading and start running. In any case, it's trivial to circumvent the wrappers with just inline JavaScript in the HTML source. This change should give us the same effect as including the wrapper code in the default content script.

Patch Set 1 #

Patch Set 2 : Wrap tabs.executeScript #

Unified diffs Side-by-side diffs Delta from patch set Stats (+76 lines, -1 line) Patch
A lib/scriptInjection.js View 1 chunk +72 lines, -0 lines 0 comments Download
M metadata.chrome View 1 chunk +3 lines, -1 line 0 comments Download
M polyfill.js View 1 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 5
Manish Jethani
April 2, 2018, 9:47 a.m. (2018-04-02 09:47:43 UTC) #1
Manish Jethani
Patch Set 1 See description. Once we've done this, we can also harden our wrappers ...
April 2, 2018, 10:09 a.m. (2018-04-02 10:09:05 UTC) #2
Manish Jethani
Should we try to land this so that we're no longer injecting code on ninety-nine ...
April 26, 2018, 3:20 p.m. (2018-04-26 15:20:48 UTC) #3
Sebastian Noack
I would rather stick with the current implementation for now, and revisit this once we ...
April 27, 2018, 1:56 p.m. (2018-04-27 13:56:41 UTC) #4
Manish Jethani
April 27, 2018, 2:02 p.m. (2018-04-27 14:02:41 UTC) #5
On 2018/04/27 13:56:41, Sebastian Noack wrote:
> I would rather stick with the current implementation for now, and revisit this
> once we can replace it with a "snippet".

Sounds good, I'll close it then.

Powered by Google App Engine
This is Rietveld