Bug 91564 – Element wants to have userAgentShadowRoot()

• Component: DOM
• Status: RESOLVED FIXED

[Hajime Morrita]

This is a follow up t bug 77936.
userAgentShadowRoot() could have an assertion to type debug field.
Also, it will make ua-shadow related code more readable.

[Shinya Kawanaka]

I think Element should have this method, right?

A lot of code use the following pattern
  shadow()->oldestShadowRoot()->...
But we should be able to do:
  userAgentShadowRoot()->...

[Shinya Kawanaka]

Created attachment 152919 [details]
Patch

[Early Warning System Bot]

Comment on attachment 152919 [details]
Patch

Attachment 152919 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13269368

[Early Warning System Bot]

Comment on attachment 152919 [details]
Patch

Attachment 152919 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13273405

[WebKit Review Bot]

Comment on attachment 152919 [details]
Patch

Attachment 152919 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13277399

[Shinya Kawanaka]

Created attachment 152930 [details]
Patch

[Eric Carlson]

Comment on attachment 152930 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=152930&action=review

This is much cleaner, thanks!

> Source/WebCore/html/HTMLMediaElement.cpp:4185
> +    if (ShadowRoot* userAgent = userAgentShadowRoot()) {
> +        Node* node = userAgent->firstChild();
> +        return node && node->isMediaControls();
> +    }

Isn't it true that userAgentShadowRoot()->firstChild() *must* be the media controls? If so, I think "&& node->isMediaControls()" could be replaced with "ASSERT(node->isMediaControls())".

[Shinya Kawanaka]

(In reply to comment #7)
> (From update of attachment 152930 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=152930&action=review
> 
> This is much cleaner, thanks!
> 
> > Source/WebCore/html/HTMLMediaElement.cpp:4185
> > +    if (ShadowRoot* userAgent = userAgentShadowRoot()) {
> > +        Node* node = userAgent->firstChild();
> > +        return node && node->isMediaControls();
> > +    }
> 
> Isn't it true that userAgentShadowRoot()->firstChild() *must* be the media controls? If so, I think "&& node->isMediaControls()" could be replaced with "ASSERT(node->isMediaControls())".

If firstChild() exists, it must be the media control, I believe.
I'll update the patch, and if it passes tests, I'll land it.

[Shinya Kawanaka]

Created attachment 153150 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 153150 [details]
Patch for landing

Clearing flags on attachment: 153150

Committed r123071: <http://trac.webkit.org/changeset/123071>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.