feat: introduce the use of reference to the helper libs
#2
Conversation
| tabId: sender.tab?.id, | ||
| frameId: sender.frameId, | ||
|
|
||
| senderDetails: sender, |
There was a problem hiding this comment.
Cannot see why I would need that, maybe we can keep it simple for now?
There was a problem hiding this comment.
Sure, only tabId and frameId are now left.
|
|
||
| reference: { | ||
| event, | ||
| message: msg, |
There was a problem hiding this comment.
looks like message.ids, hrefs and classes are already passed as arguments to getCosmeticsFilters - so no need to pass it again
There was a problem hiding this comment.
Just added a lifecycle variable from the msg object, and replaced event to frameId and processId.
| @@ -186,6 +186,10 @@ export class ElectronBlocker extends FiltersEngine { | |||
| getExtendedRules: true, | |||
| getRulesFromHostname: true, | |||
| getRulesFromDOM: false, // Only done on updates (see `onGetCosmeticFiltersUpdated`) | |||
|
|
|||
| reference: { | |||
| event, | |||
There was a problem hiding this comment.
from what I can see the only sensible reference we can pass is the url
Please don't pass the whole event - it looks like a very complicated object, may cause problems.
There was a problem hiding this comment.
Replaced the event to frameId and processId.
| @@ -179,6 +179,10 @@ export class PlaywrightBlocker extends FiltersEngine { | |||
|
|
|||
| // Will handle DOM features (see below). | |||
| getRulesFromDOM: false, | |||
|
|
|||
| reference: { | |||
| frame, | |||
There was a problem hiding this comment.
I would only pass url here
There was a problem hiding this comment.
I think the url is already passed (also, it's extracted from the frame). Removed.
| tabId: details.tabId, | ||
| frameId: details.frameId, | ||
|
|
||
| navigationDetails: details, |
There was a problem hiding this comment.
not sure how to use it, lets remove for now
There was a problem hiding this comment.
I also don't see how to use it. Removed navigationDetails.
| @@ -226,6 +226,10 @@ export class PuppeteerBlocker extends FiltersEngine { | |||
|
|
|||
| // Allows to get styles for updated DOM. | |||
| getRulesFromDOM: true, | |||
|
|
|||
| reference: { | |||
| frame, | |||
There was a problem hiding this comment.
does frame have an .id? would prefer to pass it instead of full object. again, we don't want to retain reference to it as it may cause memory leaks
There was a problem hiding this comment.
No, I don't think so. I just dropped this part.
seia-soto
changed the title
reference to the help libsreference to the helper libs
ghostery#3881 (review)