Pages using VWO.com for A/B testing delay before rendering

[jeroenvisser101]

Similar to #4402, but related to A/B testing code from VWO

Description

Sites using VWO.com for a/b testing, when loaded with shields up, will show up as a blank page while VWO waits for its experiment code to load. Only after a set timeout (which defaults to around 2 seconds), it will show the page as a fallback.

var _vwo_code=function(e){var t=e,i=2e3,n=2500,r=false,o=false,a=document;return{use_existing_jquery:function(){return r},library_tolerance:function(){return n},finish:function(){if(!o){o=true;var e=a.getElementById("_vis_opt_path_hides");if(e)e.parentNode.removeChild(e)}},finished:function(){return o},load:function(e){var t=a.createElement("script");t.src=e;t.type="text/javascript";t.innerText;t.onerror=function(){_vwo_code.finish()};a.getElementsByTagName("head")[0].appendChild(t)},init:function(){settings_timer=setTimeout("_vwo_code.finish()",i);var e=a.createElement("style"),n="body{opacity:0 !important;filter:alpha(opacity=0) !important;background:none !important;}",r=a.getElementsByTagName("head")[0];e.setAttribute("id","_vis_opt_path_hides");e.setAttribute("type","text/css");if(e.styleSheet)e.styleSheet.cssText=n;else e.appendChild(a.createTextNode(n));r.appendChild(e);this.load("//dev.visualwebsiteoptimizer.com/j.php?a="+t+"&u="+encodeURIComponent(a.URL)+"&r="+Math.random());return settings_timer}}}(visualWebsiteOptimizerId);_vwo_settings_timer=_vwo_code.init();

Steps to Reproduce

1. Open Brave, ensure shields are up.
2. Navigate to https://www.drukwerkdeal.nl (or any other site using VWO)
3. Notice the page finishes loading but then takes another 2 seconds to 'render'.

Actual result:

Page is shown after delay.

Expected result:

Page is shown immediately after loading.

Reproduces how often:

100% of the time

Brave version (brave://version info)

All channels.

Other Additional Information:

• Does the issue resolve itself when disabling Brave Shields? YES
• Does the issue resolve itself when disabling Brave Rewards? NO
• Is the issue reproducible on the latest version of Chrome? NO

Miscellaneous Information:

Calling _vwo_code.finish() signals to the page that it can be shown and is used in the VWO experiment code as fallback in catch statement.

[bsclifton]

cc: @snyderp

[jeroenvisser101]

@snyderp pattern for the script that should be replaced as polyfill is https://dev.visualwebsiteoptimizer.com/j.php?*. https://nu.nl is also a large site that uses VWO.com that you can test on. Let me know if there's anything I can do to help.

[pes10k]

This will also happen on a sig subset of these sites: https://publicwww.com/websites/dev.visualwebsiteoptimizer.com/

Triggered by how we do blocking (reporting success and returning nothing). Will come up with a polyfill now for a short term fix

[jeroenvisser101]

As far as I've been able to find out, there isn't any JS API that would call into the script loaded by j.php. I believe that if the polyfill would contain _vwo_code.finish(), possibly wrapped in try-catch to be extra safe, it would do the job.

Is there an alternative for these kinds of platforms (Google Optimize, VWO.com) that would be a better fix for the long term?

[pes10k]

One long term solution we're deploying soon is to support the uBO format for these, so that we can share effort with that project.

Longer term, we're thinking of a number of ways we can respond dynamically

[pes10k]

I put together a PR for this last night: brave/brave-core#2956

But the big problem is the current way of doing pollyfils (changing the URL of the resource to be a JS url, and having an empty body) breaks a bunch of CSP policies. @bbondy @AndriusA or @bsclifton might have an idea of how to best proceed: e.g

• whether this will be a problem w/ uBO injections [i dont think so from my read of the code…]
• if we want to start mucking w/ people's CSP policies [we probably dont…]
• if there is a way to replace the body of requests, and not just the URL, with the given polyfill decision point [🤷‍♂️]

Also alarming, since we haven't disabled CSP reports, we're probably generating lots of CSP violation reports for the existing polyfills too, which is a bad look…

[pes10k]

As an extra bit of "funny", triggering the CSP error actually fixes this problem, since it causes the "error" event on the script to trigger, instead of the load event… but, thats a bug fixing a bug…

[jeroenvisser101]

Also alarming, since we haven't disabled CSP reports, we're probably generating lots of CSP violation reports for the existing polyfills too, which is a bad look…

I think this is also true then for all uBlock users? They also use a similar approach right? With 307 internal redirects?

[jeroenvisser101]

As an extra bit of "funny", triggering the CSP error actually fixes this problem, since it causes the "error" event on the script to trigger, instead of the load event… but, thats a bug fixing a bug…

We could trigger 'any' error on the script then, at least for the cases where we know the default implementation of the script handles errors and has a fallback. Simple net::ERR_BLOCKED_BY_CLIENT would suffice. Might even be more appropriate in these cases. Would seem like it's the least invasive, since we're not introducing any 'untrusted' or foreign code into the website, we're just telling the website that the browser couldn't load these resources, which could happen in any browser for all kinds of reasons.

EDIT:

It seems uBlock is doing exactly that:

[pes10k]

@jeroenvisser101 thats what Brave used to do :) the problem is that just as many scripts operate in the opposite way, and give the negative outcome if you trigger the error path, instead of the success path (google analytics for example). Still digging / thinking what a more general solution might look like

[jeroenvisser101]

I might be wrong but maybe we could add an extra check here to match async render blocking scripts somewhere here:

https://github.com/brave/brave-core/blob/5dfea22d582c35d00da682e261f6cad9296c2e9e/browser/net/brave_ad_block_tp_network_delegate_helper.cc#L155-L188

Seems like if we can return net::OK here, we can also return net::ERR_BLOCKED_BY_CLIENT?

[pes10k]

Sure, we can def return different values ;)

The point is just that sometimes scripts do the bad thing if you block them hard (i.e. net::ERR_BLOCKED_BY_CLIENT), and sometimes they do the bad thing if you block them soft (net::OK w/ empty response body). How to "extra check… to match async render blocking scripts" in the general case is the difficult part (whether to block hard or soft) at runtime.

Current ideas (all speculative at this point) are

• offline crawling measurement and try and determine programmatically which is the better option, for which rule (on each domain?)
• some kind of limited runtime AST analysis to try and determine the above at run time (unlikely, given that, how blink is currently structured, the networking system has no idea about what element initiated the request)
• case by case after the fact / GH issue based attrition

None super appealing, but being worked on :)

[antonok-edm]

@pes10k duplicate? sounds like this has already been fixed by us returning net::ERR_BLOCKED_BY_CLIENT rather than net::OK

[pes10k]

Yep, looks fixed. I'll close. Thanks for the catch @antonok-edm