Issue 5650 - Prevent attachShadowRoot to create closed shadow root.

Issue 5650 - Prevent attachShadowRoot to create closed shadow root.

[hub, 2017-09-29 14:51:43]

I chose to actually prevent closed shadow root rather than holding onto them. It
is simpler approach.

[lainverse, 2017-09-29 15:57:35]

https://codereview.adblockplus.org/29559650/diff/29559653/inject.preload.js
File inject.preload.js (right):

https://codereview.adblockplus.org/29559650/diff/29559653/inject.preload.js#n...
inject.preload.js:146: Element.prototype._attachShadow =
Element.prototype.attachShadow;
You left unwrapped _attachShadow right in the same object with a wrapped one.
Yandex and others will just check presence of this one first and use it instead
of standard one. Better define it as a local variable here and use 'return
_attachShadow.call(this, options);' below.

BTW, it makes ABP detection ridiculously easy. Just check presence of this
function in the Element's prototype.

https://codereview.adblockplus.org/29559650/diff/29559653/inject.preload.js#n...
inject.preload.js:150: options.mode = "open";
This is indeed the easiest solution, but I think this could be considered as a
security violation in case there are banks which use shadow DOM as extra
protection.

Also, some sites may have crawler-like scripts which check presence of open
shadows and do changes there. This may break them.

Additionally this makes ABP detection easy. Create div, attach shadow, check if
shadowRoot present.

At least make a global WeakMap with all nodes which were affected and return
null in shadowRoot getters for them.

[hub, 2017-09-29 16:54:29]

I haven't addressed the second issue yet.

https://codereview.adblockplus.org/29559650/diff/29559653/inject.preload.js
File inject.preload.js (right):

https://codereview.adblockplus.org/29559650/diff/29559653/inject.preload.js#n...
inject.preload.js:146: Element.prototype._attachShadow =
Element.prototype.attachShadow;
On 2017/09/29 15:57:35, lainverse wrote:
> You left unwrapped _attachShadow right in the same object with a wrapped one.
> Yandex and others will just check presence of this one first and use it
instead
> of standard one. Better define it as a local variable here and use 'return
> _attachShadow.call(this, options);' below.
> 
> BTW, it makes ABP detection ridiculously easy. Just check presence of this
> function in the Element's prototype.

good point. done.

[Manish Jethani, 2017-09-29 17:30:40]

Is this really the solution?

If we change the behavior of attachShadow in this way, it is likely to break at
least some web pages out there.

Also, the shadow being closed is really not the issue. Even if it were open, we
would still have to query the shadow roots to find the elements we want to hide.
This means we need a way to access all the shadow roots in the document, and I
don't think there's any API for that, but I may be wrong. We could just query
every single element to find out if it has a shadow root, but that would be
infeasibly inefficient. I'm not entirely sure about this, but I think the only
way to do this is to maintain a list of all the shadow roots in the document. We
can do this by collecting the shadow roots in the attachShadow wrapper, but
without modifying the behavior (i.e. simply forward the call to the original
function).

I'm no expert on Shadow DOM, but it seems like there is no way to remove a
shadow root, which means it should be OK to hold on to all the shadow roots ever
created until the life of the document.

[Manish Jethani, 2017-09-29 17:38:19]

On 2017/09/29 17:30:40, Manish Jethani wrote:
> Is this really the solution?
> 
> If we change the behavior of attachShadow in this way, it is likely to break
at
> least some web pages out there.
> 
> Also, the shadow being closed is really not the issue. Even if it were open,
we
> would still have to query the shadow roots to find the elements we want to
hide.
> This means we need a way to access all the shadow roots in the document, and I
> don't think there's any API for that, but I may be wrong. We could just query
> every single element to find out if it has a shadow root, but that would be
> infeasibly inefficient. I'm not entirely sure about this, but I think the only
> way to do this is to maintain a list of all the shadow roots in the document.
We
> can do this by collecting the shadow roots in the attachShadow wrapper, but
> without modifying the behavior (i.e. simply forward the call to the original
> function).
> 
> I'm no expert on Shadow DOM, but it seems like there is no way to remove a
> shadow root, which means it should be OK to hold on to all the shadow roots
ever
> created until the life of the document.

Note that user style sheets will soon be available in Chrome, which means we can
probably change our implementation to use user style sheets (they apply to all
shadow DOMs in the document) instead of modifying the element's style property.
Even so, we'll still need a way to query the elements inside the shadow DOMs for
-abp-contains. We just need to find the best way to do it.

[hub, 2017-09-29 20:01:08]

This patch seems to trigger the non shadow root scenario with the test case I
testing. Like if I am on Firefox that doesn't have the shadow DOM.

In the end, this mean that we can block, but not the way intended.

I'll need to dig this further.

[Sebastian Noack, 2017-09-29 20:13:19]

As Manish already raised concerns about, breaking web APIs (for all websites) is
definitely not an option.

If we are positive that user stylesheets will make it into Chrome soon, and if
user stylesheets are applied in the Shadow DOM, then we should rather make
emulation filters inject CSS to hide those elements. This will also address
other circumvention methods and reduces side effects all along. After all this
was our plan in the first place, but besides some challenges to consider DOM
modifications, the lack of user stylesheets in Chrome was the reason we deferred
that approach.

[hub, 2017-09-29 20:23:47]

On 2017/09/29 20:13:19, Sebastian Noack wrote:
> As Manish already raised concerns about, breaking web APIs (for all websites)
is
> definitely not an option.
> 
> If we are positive that user stylesheets will make it into Chrome soon, and if
> user stylesheets are applied in the Shadow DOM, then we should rather make
> emulation filters inject CSS to hide those elements. This will also address
> other circumvention methods and reduces side effects all along. After all this
> was our plan in the first place, but besides some challenges to consider DOM
> modifications, the lack of user stylesheets in Chrome was the reason we
deferred
> that approach.

The problem was actually looking inside these elements. It is not about applying
styles.

[Manish Jethani, 2017-09-29 21:57:03]

On 2017/09/29 20:23:47, hub wrote:
> On 2017/09/29 20:13:19, Sebastian Noack wrote:
> > [...]
> >
> > If we are positive that user stylesheets will make it into Chrome soon, and
if
> > user stylesheets are applied in the Shadow DOM, then we should rather make
> > emulation filters inject CSS to hide those elements. This will also address
> > other circumvention methods and reduces side effects all along. After all
this
> > was our plan in the first place, but besides some challenges to consider DOM
> > modifications, the lack of user stylesheets in Chrome was the reason we
> deferred
> > that approach.
> 
> The problem was actually looking inside these elements. It is not about
applying
> styles.

Indeed, we still need a way to peek inside the shadow DOMs.

[Manish Jethani, 2017-09-29 23:39:34]

On 2017/09/29 17:30:40, Manish Jethani wrote:

> If we change the behavior of attachShadow in this way, it is likely to break
at
> least some web pages out there.
> [...]

After working on the core side of this, I am convinced now that we need to
override Element.attachShadow to create an open shadow root. But we must also
fake a closed shadow root. Thankfully the only difference between an open and a
closed shadow root is that some APIs don't let to access the shadow root if it's
closed. Otherwise there is no difference in behavior at all.

Here's what I came up with:

  if ("attachShadow" in Element.prototype)
  { 
    let desc = Object.getOwnPropertyDescriptor(Element.prototype,
                                               "attachShadow");
    let attachShadow = Function.prototype.apply.bind(desc.value);
    let dispatchEvent = Function.prototype.call.bind(
      EventTarget.prototype.dispatchEvent);

    desc.value = function(...args)
    {
      let shadowRootInit = args[0];
      let closed = shadowRootInit && shadowRootInit.mode == "closed";
  
      // Create an open shadow root even if the mode requested is closed.
      if (closed)
        shadowRootInit.mode = "open";

      let shadowRoot = attachShadow(this, args);

      if (closed && shadowRoot.mode == "open")
      {
        // Fake a closed shadow root by overriding the mode property.
        Object.defineProperty(shadowRoot, "mode", {
          configurable: true,
          enumerable: true,
          value: "closed"
        });
      }

      dispatchEvent(this, new RealCustomEvent("shadowAttached"));
      return shadowRoot;
    };

    Object.defineProperty(Element.prototype, "attachShadow", desc);
  }

In addition to this, we must also modify the shadow root getter so that if
thisShadowRoot.mode is "closed" then we return null (since we're faking it).

[Sebastian Noack, 2017-09-30 02:53:28]

On 2017/09/29 23:39:34, Manish Jethani wrote:
> On 2017/09/29 17:30:40, Manish Jethani wrote:
> 
> > If we change the behavior of attachShadow in this way, it is likely to break
> at
> > least some web pages out there.
> > [...]
> 
> After working on the core side of this, I am convinced now that we need to
> override Element.attachShadow to create an open shadow root. But we must also
> fake a closed shadow root. Thankfully the only difference between an open and
a
> closed shadow root is that some APIs don't let to access the shadow root if
it's
> closed. Otherwise there is no difference in behavior at all.
> 
> Here's what I came up with:
> 
>   if ("attachShadow" in Element.prototype)
>   { 
>     let desc = Object.getOwnPropertyDescriptor(Element.prototype,
>                                                "attachShadow");
>     let attachShadow = Function.prototype.apply.bind(desc.value);
>     let dispatchEvent = Function.prototype.call.bind(
>       EventTarget.prototype.dispatchEvent);
> 
>     desc.value = function(...args)
>     {
>       let shadowRootInit = args[0];
>       let closed = shadowRootInit && shadowRootInit.mode == "closed";
>   
>       // Create an open shadow root even if the mode requested is closed.
>       if (closed)
>         shadowRootInit.mode = "open";
> 
>       let shadowRoot = attachShadow(this, args);
> 
>       if (closed && shadowRoot.mode == "open")
>       {
>         // Fake a closed shadow root by overriding the mode property.
>         Object.defineProperty(shadowRoot, "mode", {
>           configurable: true,
>           enumerable: true,
>           value: "closed"
>         });
>       }
> 
>       dispatchEvent(this, new RealCustomEvent("shadowAttached"));
>       return shadowRoot;
>     };
> 
>     Object.defineProperty(Element.prototype, "attachShadow", desc);
>   }
> 
> In addition to this, we must also modify the shadow root getter so that if
> thisShadowRoot.mode is "closed" then we return null (since we're faking it).

Yes, this is the way to go. Just two more things to consider:

1. We should randomize the event name, just like we did here [1], so that
websites cannot target/detect it.
2. We should not modify objects that are passed in from the calling code.

[1]:
https://hg.adblockplus.org/adblockpluschrome/file/a5c2164d73ca/inject.preload...

[lainverse, 2017-09-30 14:06:08]

I've decided to fix issue 2 mentioned above and and implement shadowRoot getter

if ("attachShadow" in Element.prototype)
{ 
  // attachShadow function wrapper
  let attachShadowDescriptor = Object.getOwnPropertyDescriptor(
    Element.prototype, "attachShadow");
  let attachShadow =
Function.prototype.apply.bind(attachShadowDescriptor.value);
  let dispatchEvent = Function.prototype.call.bind(
    EventTarget.prototype.dispatchEvent);
  let modeOpen = { mode: "open" };
  let valueClosed = {
    configurable: true,
    enumerable: true,
    value: "closed"
  };

  attachShadowDescriptor.value = function(...args)
  {
    let closed = false;
    let options = args[0];
    if (options) // TODO: make sure 'options' are a proper Object?
    {
      closed = options.mode === "closed";
      if (closed)
        args[0] = Object.assign({}, options, modeOpen);
    }
    // Create an open shadow root even if the mode requested is closed.
    let shadowRoot = attachShadow(this, args);

    // Fake a closed shadow root by overriding the mode property.
    if (closed)
      Object.defineProperty(shadowRoot, "mode", valueClosed);

    // TODO: Randomize event name
    dispatchEvent(this, new RealCustomEvent("shadowAttached"));
    return shadowRoot;
  };

  // shadowRoot getter wrapper, return null if mode === 'closed'
  let shadowRootDescriptor = Object.getOwnPropertyDescriptor(
    Element.prototype, "shadowRoot");
  let getShadowRoot = Function.prototype.apply.bind(shadowRootDescriptor.get);

  shadowRootDescriptor.get = functin(...args)
  {
    if (this.shadowRoot && this.shadowRoot.mode === "closed")
      return null;
    return getShadowRoot(this, args);
  };

  // apply modified descriptors to Element's prototype
  Object.defineProperties(Element.prototype, {
    attachShadow: attachShadowDescriptor,
    shadowRoot: shadowRootDescriptor
  });
}

[Sebastian Noack, 2017-10-01 04:49:43]

I wonder whether we couldn't just attach the shadow root to the event, so that
we don't have to force it be left open, but can still access it, in the our code
handling listening to that event. This would to simplify the logic here
massively, and also relying on some independent code to close it later isn't
great in the first place.

[lainverse, 2017-10-02 06:41:21]

https://codereview.adblockplus.org/29559650/diff/29559687/inject.preload.js
File inject.preload.js (right):

https://codereview.adblockplus.org/29559650/diff/29559687/inject.preload.js#n...
inject.preload.js:152: return _attachShadow.call(this, options);
Arrow functions doesn't have their own 'this' and 'arguments'.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/A...

[Manish Jethani, 2017-10-02 07:28:48]

On 2017/10/01 04:49:43, Sebastian Noack wrote:
> I wonder whether we couldn't just attach the shadow root to the event, so that
> we don't have to force it be left open, but can still access it, in the our
code
> handling listening to that event. This would to simplify the logic here
> massively, and also relying on some independent code to close it later isn't
> great in the first place.

This is not an option unfortunately because we cannot share object references
across trust boundaries. We are overriding Element.attachShadow in the code
injected into the page, whereas the ElemHideEmulation instance is in the content
script.

[Sebastian Noack, 2017-10-03 00:54:25]

On 2017/10/02 07:28:48, Manish Jethani wrote:
> On 2017/10/01 04:49:43, Sebastian Noack wrote:
> > I wonder whether we couldn't just attach the shadow root to the event, so
that
> > we don't have to force it be left open, but can still access it, in the our
> code
> > handling listening to that event. This would to simplify the logic here
> > massively, and also relying on some independent code to close it later isn't
> > great in the first place.
> 
> This is not an option unfortunately because we cannot share object references
> across trust boundaries. We are overriding Element.attachShadow in the code
> injected into the page, whereas the ElemHideEmulation instance is in the
content
> script.

Yeah, you are right. But wait, so we'd still leave the shadow root open,
essentially breaking DOM functionality? I thought we agreed that this isn't an
option, or do I miss anything?

[Manish Jethani, 2017-10-03 10:39:06]

On 2017/10/03 00:54:25, Sebastian Noack wrote:
> On 2017/10/02 07:28:48, Manish Jethani wrote:
> > On 2017/10/01 04:49:43, Sebastian Noack wrote:
> > > I wonder whether we couldn't just attach the shadow root to the event, so
> that
> > > we don't have to force it be left open, but can still access it, in the
our
> > code
> > > handling listening to that event. This would to simplify the logic here
> > > massively, and also relying on some independent code to close it later
isn't
> > > great in the first place.
> > 
> > This is not an option unfortunately because we cannot share object
references
> > across trust boundaries. We are overriding Element.attachShadow in the code
> > injected into the page, whereas the ElemHideEmulation instance is in the
> content
> > script.
> 
> Yeah, you are right. But wait, so we'd still leave the shadow root open,
> essentially breaking DOM functionality? I thought we agreed that this isn't an
> option, or do I miss anything?

I addressed this here:

https://codereview.adblockplus.org/29559650/#msg11

Our only option, it seems, is to create an open shadow root, but then we
override the "mode" property of the ShadowRoot instance to return "closed"
instead of "open". In the Element.shadowRoot getter, we return null if the mode
is "closed". We may have to override other APIs as well.

[lainverse, 2017-10-03 10:57:27]

Technically there are more options. They are more complicated, though.
1. Keep list of roots in context of current window and run code which handles
`:-abp-...` filters or at least parts of there.
2. Pass all newly created shadow roots to ABP. The main idea is to create long
random text property in the window, assign a link to shadowRoot there, fire
event for ABP to pick it up and delete that property immediately. The main issue
here is to do it fast enough to not let page detect it and change value.

Both options won't require wrapping a shadowRoot getter, redefine 'mode'
property and shouldn't have any effect on the API.

[Manish Jethani, 2017-10-03 11:20:15]

On 2017/10/03 10:57:27, lainverse wrote:
> [...]
> 2. Pass all newly created shadow roots to ABP. The main idea is to create long
> random text property in the window, assign a link to shadowRoot there, fire
> event for ABP to pick it up and delete that property immediately. The main
issue
> here is to do it fast enough to not let page detect it and change value.

Again, trust boundaries. If you add anything to the window object inside the
page, it won't be visible in the window object of the content script.

[lainverse, 2017-10-03 11:41:46]

Not sure how Tampermonkey/Greasemonkey works does this, but they both provide
UnsafeWindow object to user scripts which allows user scripts to cross this
boundary.

[lainverse, 2017-10-03 14:32:43]

On 2017/10/03 11:41:46, lainverse wrote:
> Not sure how Tampermonkey/Greasemonkey works does this, but they both provide
> UnsafeWindow object to user scripts which allows user scripts to cross this
> boundary.

Example (save it in Tampermonkey, refresh this page and look into console):
// ==UserScript==
// @name         TestScript
// @namespace    http://tampermonkey.net/
// @version      0.1
// @description  try to take over the world!
// @author       You
// @match        https://codereview.adblockplus.org/29559650/
// @grant        unsafeWindow
// ==/UserScript==

(function() {
    'use strict';

    let scr = document.createElement('script');
    scr.textContent = `
    let div = document.querySelector('#issue-star-29559650');
    window.someShadow = div.attachShadow({mode:'closed'});
    `;
    document.documentElement.appendChild(scr);
    setTimeout(()=>console.log(window.someShadow,
unsafeWindow.someShadow),1000);
})();

Output in the console:
> undefned #shadow-root (closed)

As you can see even though window.someShadow didn't cross the boundary
unsafeWindow.someShadow did.

[Sebastian Noack, 2017-10-03 18:07:38]

That is interesting. It seems that Tampermonkey indeed found a way to access the
window from the page context directly. Unfortunately, Tampermonkey isn't open
source anymore, and if I grep for "unsafeWindow" through their obfuscated code I
don't find any meaningful matches. The trick [1] Tampermonkey did in their last
version the source code has been published of does no longer seem to work.

[1] https://github.com/Tampermonkey/tampermonkey/blob/master/src/content.js#L477

[Manish Jethani, 2017-10-03 18:15:20]

On 2017/10/03 18:07:38, Sebastian Noack wrote:
> That is interesting. It seems that Tampermonkey indeed found a way to access
the
> window from the page context directly. Unfortunately, Tampermonkey isn't open
> source anymore, and if I grep for "unsafeWindow" through their obfuscated code
I
> don't find any meaningful matches. [...]

AFAIK unsafeWindow is legacy and was never supported in Chrome, also no longer
supported in Firefox.

[Sebastian Noack, 2017-10-03 18:35:36]

On 2017/10/03 18:15:20, Manish Jethani wrote:
> On 2017/10/03 18:07:38, Sebastian Noack wrote:
> > That is interesting. It seems that Tampermonkey indeed found a way to access
> the
> > window from the page context directly. Unfortunately, Tampermonkey isn't
open
> > source anymore, and if I grep for "unsafeWindow" through their obfuscated
code
> I
> > don't find any meaningful matches. [...]
> 
> AFAIK unsafeWindow is legacy and was never supported in Chrome, also no longer
> supported in Firefox.

Apparently unsafeWindow is working in Tampermonkey on Chrome, it might have some
limitations compared to Greasemonkey though. I don't see any indication that it
is deprecated, but even if it would be that doesn't matter. The question is how
it is working, and whether we can do something similar here. Note that
unsafeWindow isn't provided by Chrome but by Tampermonkey, and in order to do so
there seems to be a way to cross trust boundaries.

[Sebastian Noack, 2017-10-03 19:46:02]

It seems that the Tampermonkey scripts never actually run in the context of the
content script, but in some kind of sandbox created by Tampermonkey. Otherwise
if two scripts would run on the same page these could interfere with each other,
or even mess with Tampermonkey itself. Another problem for Tampermonkey is that
you cannot run JavaScript code from a string in the context of the content
script, but in the context of the page you can. So I guess, Tampermonkey scripts
always run in the context of the page, but with a faked window object, and the
original window object is just reassigned to unsafeWindow if requested. However,
this wouldn't help us here. So never mind.

Still, just leaving the shadow DOM open isn't great. At the very least this will
show incorrect information in the developer tools, even if we patch the
shadowRoot getter and the mode property of the ShadowRoot instance. I wonder
whether Chromium should just expose closed shadow roots through the shadowRoot
property in content scripts. After all extensions are meant to extend/intercept
normal web page behavior, and therefore accessing closed shadow roots seems a
legit use case.

[Manish Jethani, 2017-10-03 23:06:09]

On 2017/10/03 19:46:02, Sebastian Noack wrote:
> [...]
>
> Still, just leaving the shadow DOM open isn't great. At the very least this
will
> show incorrect information in the developer tools, even if we patch the
> shadowRoot getter and the mode property of the ShadowRoot instance. I wonder

Developer tools is a small user base, right?

I would be annoying for developers, but they can just disable Adblock Plus on
whatever site they're testing.

> whether Chromium should just expose closed shadow roots through the shadowRoot
> property in content scripts. After all extensions are meant to
extend/intercept
> normal web page behavior, and therefore accessing closed shadow roots seems a
> legit use case.

The browser could help here in one of the following ways:

 1.  In content scripts the shadowRoot property of the Element object could
return the shadow root instead of null even when the mode is "closed"
 2.  When Element.attachShadow is called, the element could dispatch an event
with the detail property set to the shadow root
 3.  MutationObserver could take a "shadow" option, which if set would cause it
to observe changes within shadow DOMs (expensive)
 4.  MutationObserver could take a "shadow" option, which if set would cause it
to observe changes to an element's shadow root

Based on my reading, I think the 4th option would be best. MutationRecord.type
would be "shadow", MutationRecord.target would be the element,
MutationRecord.shadow (new property) would be the shadow root, and
MutationRecord.oldValue would be the old shadow root.

[Sebastian Noack, 2017-10-04 00:11:49]

On 2017/10/03 23:06:09, Manish Jethani wrote:
> Developer tools is a small user base, right?

It's probably not the majority of users, but probably not just 1% either.
Technical users are generally more likely to install an ad blocker. Moreover,
there also have been legal/copyright shenanigans based on how the representation
of the DOM, as seen in the developer tools, is changed, in other cases.

> I would be annoying for developers, but they can just disable Adblock Plus on
> whatever site they're testing.

They cannot. We'd patch the DOM API on every website our content script runs on.
We cannot first check whether the website is whitelisted because the page's code
might already run before we get an asynchronous response.

> The browser could help here in one of the following ways:
> 
>  1.  In content scripts the shadowRoot property of the Element object could
> return the shadow root instead of null even when the mode is "closed"
>  2.  When Element.attachShadow is called, the element could dispatch an event
> with the detail property set to the shadow root
>  3.  MutationObserver could take a "shadow" option, which if set would cause
it
> to observe changes within shadow DOMs (expensive)
>  4.  MutationObserver could take a "shadow" option, which if set would cause
it
> to observe changes to an element's shadow root
> 
> Based on my reading, I think the 4th option would be best. MutationRecord.type
> would be "shadow", MutationRecord.target would be the element,
> MutationRecord.shadow (new property) would be the shadow root, and
> MutationRecord.oldValue would be the old shadow root.

#2, #3 and #4 would either require extending the Web Components standard or
implementing non-standard behavior. For #1, on the other hand, the standard
doesn't need to be considered, as it only effects browser extensions which
aren't covered by the standard.

Also #2, #3 and #4 would leak the shadow DOM, defeating the purpose of having
closed shadow roots in the first place, which is that they are only accessible
by the code that creates them. This would no longer be true if they are
broadcasted through events or mutation records.

Also see what Hayato, who implemented Shadow DOM in Chromium, says:
https://github.com/w3c/webcomponents/issues/378#issuecomment-179596975

[Manish Jethani, 2017-10-04 04:23:15]

On 2017/10/04 00:11:49, Sebastian Noack wrote:
> On 2017/10/03 23:06:09, Manish Jethani wrote:
> 
> [...]
> 
> > I would be annoying for developers, but they can just disable Adblock Plus
on
> > whatever site they're testing.
> 
> They cannot. We'd patch the DOM API on every website our content script runs
on.
> We cannot first check whether the website is whitelisted because the page's
code
> might already run before we get an asynchronous response.

I see.

> > The browser could help here in one of the following ways:
> > 
> >  1.  In content scripts the shadowRoot property of the Element object could
> > return the shadow root instead of null even when the mode is "closed"
> >  2.  When Element.attachShadow is called, the element could dispatch an
event
> > with the detail property set to the shadow root
> >  3.  MutationObserver could take a "shadow" option, which if set would cause
> it
> > to observe changes within shadow DOMs (expensive)
> >  4.  MutationObserver could take a "shadow" option, which if set would cause
> it
> > to observe changes to an element's shadow root
> > 
> > Based on my reading, I think the 4th option would be best.
MutationRecord.type
> > would be "shadow", MutationRecord.target would be the element,
> > MutationRecord.shadow (new property) would be the shadow root, and
> > MutationRecord.oldValue would be the old shadow root.
> 
> #2, #3 and #4 would either require extending the Web Components standard or
> implementing non-standard behavior. For #1, on the other hand, the standard
> doesn't need to be considered, as it only effects browser extensions which
> aren't covered by the standard.
> 
> Also #2, #3 and #4 would leak the shadow DOM, defeating the purpose of having
> closed shadow roots in the first place, which is that they are only accessible
> by the code that creates them. This would no longer be true if they are
> broadcasted through events or mutation records.
> 
> Also see what Hayato, who implemented Shadow DOM in Chromium, says:
> https://github.com/w3c/webcomponents/issues/378#issuecomment-179596975

OK, that makes sense.

If we don't have any other ideas, we should close this review and file an issue
with Chromium to allow access to closed shadow roots from extensions. Meanwhile
it's back to the drawing board for #5650.

[Sebastian Noack, 2017-10-04 05:08:46]

On 2017/10/04 04:23:15, Manish Jethani wrote:
> If we don't have any other ideas, we should close this review and file an
issue
> with Chromium to allow access to closed shadow roots from extensions.
Meanwhile
> it's back to the drawing board for #5650.

Yes, filing a Chromium bug with our suggestion (and potentially writing a patch)
should be the next step. Even if we missed something so far, this would still be
the best solution for us. In case we don't get that change into Chrome, and
don't come up with a better workaround, we can still reconsider your approach,
dependent on how critical we consider this issue.