Issue 3699 - Patch DOM API in order to prevent disabling the injected stylesheet

Issue 3699 - Patch DOM API in order to prevent disabling the injected stylesheet

[Sebastian Noack, 2016-03-18 17:36:15]

Given how many ways there are to cause an element being removed from the DOM
(patch set #1 didn't cover all of them), and also considering performance
implications when patching the DOM, I decided that using a MutationObserver is
actually better. But while bringing back the the MutationObserver, I simplified
the approach and made it more reliable, by simply reinjecting our stylesheet
where it was.

In addition, we still patch DOM APIs, but only those that cannot be covered by a
MutationObserver. Also I tidied up that code to avoid repeated logic.

[Sebastian Noack, 2016-03-18 17:58:54]

https://codereview.adblockplus.org/29338588/diff/29338656/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29338588/diff/29338656/include.preload.js#...
include.preload.js:361: '             
document.documentElement.shadowRoot.getElementById("' + id + '");',
(Yes, I know that this line is 6 characters too long. But I don't see anyway to
make it shorter without impairing readability.)

[kzar, 2016-03-18 19:18:32]

I found this code kind of confusing to understand but I think that's because the
problem is messy rather than your solution. I couldn't see any way to make it
much better.

LGTM

https://codereview.adblockplus.org/29338588/diff/29338656/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29338588/diff/29338656/include.preload.js#...
include.preload.js:361: '             
document.documentElement.shadowRoot.getElementById("' + id + '");',
On 2016/03/18 17:58:54, Sebastian Noack wrote:
> (Yes, I know that this line is 6 characters too long. But I don't see anyway
to
> make it shorter without impairing readability.)

Acknowledged.

https://codereview.adblockplus.org/29338588/diff/29338656/include.preload.js#...
include.preload.js:373: for (var j = 0; j < methods.length; j++)
Nit: Just re-use the existing `i` variable instead?

[Sebastian Noack, 2016-03-18 19:23:55]

https://codereview.adblockplus.org/29338588/diff/29338656/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29338588/diff/29338656/include.preload.js#...
include.preload.js:373: for (var j = 0; j < methods.length; j++)
On 2016/03/18 19:18:32, kzar wrote:
> Nit: Just re-use the existing `i` variable instead?

I'm not sure whether it's a good idea to make the code here depend on a
unrelated variable declared in the headers of the previous loop.