Use Shadow DOM for element hiding

Adblock Plus breaks web pages that rely on the order of their own <style> tags e.g. by using document.styleSheets as currently reported by #309. However we can use Shadow DOM in order to hide our stylesheet from the web page.

[Sebastian Noack, 2014-04-15 09:58:06]

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
File include.preload.js (left):

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
include.preload.js:26: elemhideElt.parentNode.removeChild(elemhideElt);
Storing the inserted <style> tag into a global variable in order to remove it
everytime setElemhideCSSRules() is called seems unneeded to me, since it is only
called once or did I missed something?

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
File include.preload.js (right):

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
include.preload.js:28: if (selectors.length == 0)
Skip inserting a style tag, if there are no element hiding rules for the current
website. Previously we only bailed out when the website itself is whitelisted.

[Wladimir Palant, 2014-04-15 13:13:14]

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
File include.preload.js (left):

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
include.preload.js:26: elemhideElt.parentNode.removeChild(elemhideElt);
On 2014/04/15 09:58:06, Sebastian Noack wrote:
> Storing the inserted <style> tag into a global variable in order to remove it
> everytime setElemhideCSSRules() is called seems unneeded to me, since it is
only
> called once or did I missed something?

I somewhat remember that AdThwart had issues with content scripts being applied
multiple times for some reason. I am fairly certain that this code is safe to
remove now.

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
File include.preload.js (right):

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
include.preload.js:28: if (selectors.length == 0)
On 2014/04/15 09:58:06, Sebastian Noack wrote:
> Skip inserting a style tag, if there are no element hiding rules for the
current
> website. Previously we only bailed out when the website itself is whitelisted.

That doesn't make much difference in reality - if the user has a filter list and
didn't whitelist the website then some element hiding rules will most certainly
apply to every domain. Still nothing wrong with the change of course.

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
include.preload.js:39: container = container.createShadowRoot();
Given that this standard is still very much in flux we should not expect our
code to work with the final version of Web Components. It should be better to
check for webkitCreateShadowRoot only, at least for now.

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
include.preload.js:41: container = container.webkitCreateShadowRoot();
Please add a <shadow> element to our shadow root to make sure any content
defined by the document or other shadow roots is still in the DOM tree.

Interestingly, Chrome doesn't seem to implement
HTMLShadowElement.olderShadowRoot yet. This means that by adding one more shadow
root with only the <shadow> element we can protect our style element from
website accesses. Not sure whether this is worth doing.

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
include.preload.js:45: container.appendChild(elemhideElt);
Will these styles affect the regular elements or only the shadow DOM? From
http://www.w3.org/TR/shadow-dom/#styles it seems that the latter is the case.
That was actually the reason why we dismissed using Shadow DOM for element
hiding earlier.

It seems that in order to have the styles apply to the document we have to
create the shadow root on the document element and then use the
::-webkit-distributed() pseudo-element for each selector. I wonder whether we
can measure the overhead (CPU time and memory) caused by this.

[Sebastian Noack, 2014-04-15 19:21:54]

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
File include.preload.js (right):

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
include.preload.js:28: if (selectors.length == 0)
On 2014/04/15 13:13:14, Wladimir Palant wrote:
> On 2014/04/15 09:58:06, Sebastian Noack wrote:
> > Skip inserting a style tag, if there are no element hiding rules for the
> current
> > website. Previously we only bailed out when the website itself is
whitelisted.
> 
> That doesn't make much difference in reality - if the user has a filter list
and
> didn't whitelist the website then some element hiding rules will most
certainly
> apply to every domain. Still nothing wrong with the change of course.

I am aware that at least when using EASYList there are always some element
hiding filters that are included on all websites. However handling that case
doesn't add any complexity, and IMHO it makes the API even more simple and
robust against wrong assumptions when always an array is returned.

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
include.preload.js:39: container = container.createShadowRoot();
On 2014/04/15 13:13:14, Wladimir Palant wrote:
> Given that this standard is still very much in flux we should not expect our
> code to work with the final version of Web Components. It should be better to
> check for webkitCreateShadowRoot only, at least for now.

Done.

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
include.preload.js:41: container = container.webkitCreateShadowRoot();
On 2014/04/15 13:13:14, Wladimir Palant wrote:
> Please add a <shadow> element to our shadow root to make sure any content
> defined by the document or other shadow roots is still in the DOM tree.
> 
> Interestingly, Chrome doesn't seem to implement
> HTMLShadowElement.olderShadowRoot yet. This means that by adding one more
shadow
> root with only the <shadow> element we can protect our style element from
> website accesses. Not sure whether this is worth doing.

Done.

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
include.preload.js:45: container.appendChild(elemhideElt);
On 2014/04/15 13:13:14, Wladimir Palant wrote:
> Will these styles affect the regular elements or only the shadow DOM? From
> http://www.w3.org/TR/shadow-dom/#styles it seems that the latter is the case.
> That was actually the reason why we dismissed using Shadow DOM for element
> hiding earlier.
> 
> It seems that in order to have the styles apply to the document we have to
> create the shadow root on the document element and then use the
> ::-webkit-distributed() pseudo-element for each selector. I wonder whether we
> can measure the overhead (CPU time and memory) caused by this.

You are right, it actually didn't worked with the previous patch. I confused
element hiding with element collapsing when I tested it.

But after using the document element as shadow root, adding a <shadow> node to
it and wrapping the selectors with ::-webkit-distributed() it works perfectly.

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
include.preload.js:50: if (!elt.sheet)
I also got rid of that code. The sheet property is present as soon as you insert
the element into the DOM. It works even with Safari 5.1, which is the oldest
webkit browser I could find for download. Do you remember why we had that code
in the first place?

[Wladimir Palant, 2014-04-15 21:19:47]

This is LGTM if you verify that it works in Chrome 19. We also need to check how
the memory usage changes with this change, if webpages take up much more memory
now we might have to revert it. Ideally we would measure the impact on the
processing time as well, if the stylesheet applies synchronously that might
work.

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
File include.preload.js (right):

http://codereview.adblockplus.org/5989914281771008/diff/5629499534213120/incl...
include.preload.js:50: if (!elt.sheet)
I didn't remember but hg blame points to
https://adblockplus.org/forum/viewtopic.php?t=8834. As with any issues that
cannot be reproduced, it is really hard to tell whether the issue is gone.
However, it seems that this was running on a timeout even before the change in
question - so older Chrome versions weren't initializing the element.sheet
property immediately.

Please verify that your code works in Chrome 19. You can get Chromium 19 (trunk
build, right before the release branch was created) from
https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.htm...
(builds for other operating systems are available as well).