Ublock: Question: does this code ever execute?

[dhowe]

Read first: https://github.com/gorhill/uBlock/blob/master/CONTRIBUTING.md

Describe the issue

To be clear, this is not a bug report, but only a question about a possible issue. In my tests, the conditional here, never executes, even when vAPI.skipCosmeticFiltering is true in contentscript-start.js. Is this the intended behavior, or is there a test case I have missed?

[lewisje]

Do you mean that the process of setting up cosmetic filtering happens anyway, even if vAPI.skipCosmeticFiltering is true? Have you verified that this property is still true in contentscript-end.js? What is supposed to happen is an early return from that IIFE.

[dhowe]

I can't find a case where that early return happens. When I add the following line
AFTER that conditional:

` console.log('CosmeticFilters', vAPI.skipCosmeticFiltering, vAPI);

I see the following in the console:

[lewisje]

Where, precisely, did you put that line?

Also, I inspected the content script as injected by the extension, placing one breakpoint at the conditional (around line 300) and another somewhere inside (at the idsFromNodeList(document.querySelectorAll('[id]')); line, around line 800); with "Disable cosmetic filtering" on, only the first breakpoint was hit, while with it off, both were hit.

[dhowe]

line 304.

but does that inspector data not seem strange? That vAPI.skipCosmeticFiltering prints as 'undefined', but then displays, inside the object, as 'true'...

[lewisje]

Okay, I wasn't sure whether you meant "right after the header of the if block" or "right after the entire if block" and now I know you mean the latter.

My best guess at this point is that calls to console.log are somehow hoisted, and that Inspector data is indeed strange; I haven't bothered to directly tinker with the extension to see for myself though.

Do you see the same problems if you set breakpoints, as I did?

[dhowe]

Setting breakpoints at 300 and 305 causes the problem to disappear, and the conditional to work correctly (perhaps b/c some other async call has time to complete).

However, if you set a single breakpoint at 305, for a page on which cosmeticFiltering is disabled, you see the problem quite clearly: the early-return does not happen.

[lewisje]

If I try to set a breakpoint at 305 (which is a comment), the Web Inspector actually sets it at 365, at the end of the IIFE, which will definitely be reached (specifically, it's where the function, having been set up, is immediately executed); try setting the breakpoint at 311 (which, like 300, is the header of an if block), the first line after that first if block where the Inspector didn't shift it over.

I have tested disabling cosmetic filtering per site and verified that it sets vAPI.skipCosmeticFiltering to true, just as toggling the global setting does.

[dhowe]

hideElements() is called later, but I think we want to test directly after the conditional. Here is my code:

(function() {
    if ( vAPI.skipCosmeticFiltering ) {
        console.debug('Abort cosmetic filtering');
        return;
    }

    console.debug('Start cosmetic filtering',vAPI.skipCosmeticFiltering, vAPI); // SET BREAKPOINT
    ...

[dhowe]

Alternatively you can set it at 403 (which should not execute if cosmetic-filters are disabled)...

[lewisje]

I didn't notice that 403 was the first line to execute, other than a var statement that assigns a function value, after that if block; the first such line I noticed was 800, after most of the functions were set up.

(All of my references to line numbers were to the original code, because I have not bothered to modify the extension's code even to add or uncomment logging statements.)

[dhowe]

If you set breakpoints at both 403 and 800 (in the original code), you will see that 403 is reached first.

Regardless, neither should be reached when vAPI.skipCosmeticFiltering is true.

[gorhill]

It does execute if I set the no-cosmetic-filtering switch to true for a site.

[gorhill]

but does that inspector data not seem strange?

When you expand the content of an object at the console, it will report the current state, not the state at log time. So it appears you have a case where skipCosmeticFiltering was not yet set in contentscript-start.js (possibly because waiting for answer from extension core code), while when you expanded the log output, by then the value was set.

[gorhill]

Ok so this is probably what is happening on your side, simplified, re. skipCosmeticFiltering:

• contentscript-starts.js sends message to core, to query about the value of skipCosmeticFiltering.
• contentscript-end.js starts to execute before contentscript-starts.js receives the answer.
• contentscript-end.js sees skipCosmeticFiltering as undefined.
• contentscript-starts.js finally receives the value of skipCosmeticFiltering.

You have a specific repro case for this? I would like to be able to reproduce.

Note that I ran into similar issue in the past, where there would be issues if contentscript-end.js outran contentscript-start.js (baa0fee0f3e5f09bc8e216f5a56879b6850a2f51).

[lewisje]

FWIW when I was doing my own testing, I didn't even see contentscript-start.js in the set of content scripts; now on my work computer, I don't see either contentscript-start.js or contentscript-end.js (both computers run Chrome 51 x64 on Windows 10 Pro, both stable builds, and in both cases, I tested on this issue thread). I guess TIL a bit about how content scripts work.

@dhowe, what I was saying was that I didn't notice that one function call in a sea of var statements, because I wasn't looking carefully enough.

[gorhill]

I didn't even see contentscript-start.js in the set of content scripts

You need to force a reload of a page once you open the dev console. Chromium will flush out unused code from memory, and contentscript-start.js has a one-time use, it does not hook itself to any long term listener. Once it receives its answer from the extension core, the code become unused, hence garbage-collected by Chromium.

[dhowe]

here's a simple repro case
notice that the ad is correctly shown when cosmetic filters are disabled, however the conditional at line 300 doesn't execute

[gorhill]

I can reproduce with this case. The page is so simple that the page's DOMContentLoaded (at which time the browser loads contentscript-end.js) is triggered before uBO's contentscript-start.js receives an answer from the core.

All this works fine though, since cosmetic filtering is also skipped when the core processes the request from contentscript-end.js.

So this is a case where a web page is so simple that contentscript-end.js outrun the completion of contentscript-start.js. The skipCosmeticFiltering property is to avoid _as much as possible_ useless work in contentscript-end.js. In the repro case, the useless work is not skipped, but it is not really an issue since the page is so simple. With more complex pages, skipCosmeticFiltering will likely be always properly set by the time contentscript-end.js executes.

[gorhill]

I will assume the question was answered.

[dhowe]

Indeed, thanks much for sorting through it