Implement ElectronBlocker abstraction

[sentialx]

Enable full adblocking in Electron with only few lines of code:

import { ElectronBlocker } from '@cliqz/adblocker';
import { session } from 'electron';

// Create blocker instance
const blocker = ElectronBlocker.parse('||tracker.com'); // replace with desired subscriptions
await blocker.enableBlockingInSession(session.defaultSession);

[remusao]

Thank you so much for contributing! As far as I can see the changes look great but ideally we would find a way to get type definitions for Electron without having to bundle the full package. Do you know if that is possible? Otherwise we could define these in the source code directly and make sure they are compatible with electron upstream via the example/electron?

[sentialx]

Electron has its own type definitions and @types/electron package is outdated. I think we can add Electron to rollup external

[remusao]

I'm wondering if this could cause issues for people using the adblocker without having electron installed. Since it's a dev dependency but it's still required from the bundle. Also not sure if the type definitions will be re-exported by TypeScript.

[sentialx]

I implemented some cosmetic filtering, but it doesn't inject in all frames. I think you will need to check it.

[sentialx]

I have no idea how to separate src/electron/content.ts to a dist/content.js file.

[remusao]

Few thoughts, wondering if in the future it would not make sense to have sub-packages for different environments (maybe managed with lerna, i see it's a pretty common practice): @cliqz/adblocker, @cliqz/adblocker-webextension, @cliqz/adblocker-puppeteer, @cliqz/adblocker-electron; which would give a bit more freedom with dependencies. At the moment it was kind of fine since we could get only the types for different platforms but Electron seem to require a tighter integration.

[remusao]

@sentialx the mono-repo PR was merged so we should be able to move forward with the electron wrapper. I cannot guarantee that it will be 100% smooth as I'm still figuring-out the best workflow but I'm happy to improve the tooling/flow as we go. What do you think?

[sentialx]

@remusao Sorry for the delay. Is there a way to separate packages/adblocker-electron/content.ts in rollup to build a separate js file?

[remusao]

@remusao Sorry for the delay. Is there a way to separate packages/adblocker-electron/content.ts in rollup to build a separate js file?

No worries at all! Sure you can, if your rollup exports an array, you can define several configs (with different inputs and outputs). One such example would be: https://github.com/cliqz-oss/adblocker/blob/master/packages/adblocker/rollup.config.ts (another package in this repository); this way you could create a bundle for content and another for main process.

I noticed that you added by mistake the dist and build folders, could you maybe remove them, as it makes it harder to review the actual changes.

[remusao]

You should also be able to remove package-lock.json completely. Since the switch to mono-repo structure yarn is needed to get access to the workspace feature it provides. So the only lock file needed is yarn.lock at the root of the repository.

[sentialx]

The preload script is being injected before the page load, so in main process it's correct. However I'm not sure about the MutationObserver and messaging.

[remusao]

The preload script is being injected before the page load, so in main process it's correct. However I'm not sure about the MutationObserver and messaging.

Makes sense. Is there a way to wait for DOM to be ready for injection maybe? Also, do you know if executeJavaScript and insertCSS can be called from renderer or main process interchangeably? Or will they take effect in different contexts?

[sentialx]

Is there a way to wait for DOM to be ready for injection maybe?

In webContents there's a dom-ready event.

Also, do you know if executeJavaScript and insertCSS can be called from renderer or main process interchangeably?

In renderer process you can call webFrame.insertCSS and webFrame.executeJavaScript.
In main process the only difference is that you need webContents instead of a webFrame.

[remusao]

@sentialx I've been investigating a bit further and gained some extra insight. It seems that when a new frame/content context is created and a page loads in it, sending a message to early might result in this message being dropped completely. Basically if:

1. new URL is loaded in window
2. preload script is injected and starts to run
3. preload script sends a message to main process to request cosmetics
4. main process replies with cosmetics (this is dropped most of the time because it happens to fast)

So we might need to way "a bit". Now waiting for dom-ready (or DOMContentLoaded, I think it's the same event right?) will be too late in some cases: ideally we want to start injecting as soon as possible (as long as the content context is ready to receive messages).

So now I'm not sure if we should just introduce an arbitrary delay before answering from main process, or if we could perform a preliminary handshake (at the cost of increased latency); like:

1. preload script sends a message to main process
2. main process sends the same message to content every N milliseconds, until it receives a reply
3. preload script sends acknowledgement to main process whenever it receives the first message to signal it's fully operational.

What do you think? I will play a bit more with this tomorrow.

[sentialx]

How is this possible, the renderer process loses the message sent from main process by request? I think it's something wrong with getting the styles to inject.

[remusao]

How is this possible, the renderer process loses the message sent from main process by request? I think it's something wrong with getting the styles to inject.

What I did is this:

1. console log when main process calls e.sender.send(...) (or e.reply(...))
2. console log all messages received by renderer from main process

What I observed after starting the example multiple times:

• the first message (sent before DOMContentLoaded) is received only part of the time
• often, it is not received at all in renderer (but the sending is performed in main process)
• subsequent messages from main to renderer seem to all be received.

Which is why I concluded that it might be due to a timing issue. Not sure if setting-up the ipc communication between processes might take some time?

[remusao]

Great work @sentialx! It's getting really close, very exciting. I had a few comments while reviewing the changes again. Let me know what you think.

[remusao]

Awesome, thanks a lot again! Looks very good. I'm going to give it a run on a list of websites where I know there are tricky cases for the adblocker to make sure everything works as expected (CSP headers, injections, etc.). Will merge after that.

[remusao]

We can remove & { action?: string }

[remusao]

In which case can sender be falsy?

[sentialx]

Well, that's a safe check whether it's not null or undefined.

[remusao]

Looks great. I tried it a bit locally and things seem to work fine :D (just had to fix something related to CSP injection).

[remusao]

⚡ 🚀 Nice work! It was a great experience and I learned a lot thanks to you. Thanks again for your hard work. I'm going to prepare a release soon so that we can start using this new abstraction in the wild.

[sentialx]

Thanks. Will it be a good idea to add a blocked-ad event to help counting blocked ads?

[remusao]

Thanks. Will it be a good idea to add a blocked-ad event to help counting blocked ads?

Yes, actually I was thinking that we could have a common mechanism to WebExtensionBlocker, PuppeteerBlocker and ElectronBlocker; to avoid repetition. It could even be implemented in @cliqz/adblocker (core package) in the FiltersEngine class so that all sub-classes benefit from this. I was not 100% sure about the API, and how much information we want to expose. There are multiple events that we could want to keep track of:

• requests blocked
• requests redirected
• requests which could have been blocked but benefited from an exception
• CSP headers injections
• events related to cosmetics injections: scripts injected, styles injected

Depending on the project and use-case, you might want these information or not. Maybe a simple counter of "requests altered" is enough. But before implementing such mechanism, I'd like to answer these questions so that we build something solid which can accommodate multiple use-cases.

In terms of implementation details, it could be using some EventEmitter (maybe FiltersEngine could implement this API and define events for each case we are interested in), or using some callbacks. I'd also like to make sure the performance over-head is as small as possible. Ideally, the overhead is zero is no-one is listening to events, and negligible when events are consumed.

Let's continue the discussion in a separate issue. I'd love to get your thoughts about this.