Changeset 122816 in webkit

Timestamp:

Jul 17, 2012 12:26:52 AM (12 years ago)

Author:

shinyak@chromium.org

Message:

HTMLMediaElement should not use Element::ensureShadowRoot()
​https://bugs.webkit.org/show_bug.cgi?id=77936

Reviewed by Hajime Morita.

Source/WebCore:

a video element and an audio element add UserAgentShadowRoot dynamically, and they assume that it's the oldest ShadowRoot.
However an AuthorShadowRoot could be added by a user before a video element and an audio element add UserAgentShadowRoot.
It breaks the assumption that the UserAgentShadowRoot is the oldest.

If the UserAgentShadowRoot is not the oldest, the AuthorShadowRoot a page author added might be ignored.
Since the timing to add UserAgentShadowRoot is not known by a user, the fact that UserAgentShadorRoot is
not the oldest will cause inconsistent behavior.

Adding AuthorShadowRoot to a video element and an audio element is allowed by this patch.

Test: fast/dom/shadow/shadowdom-for-media.html

• dom/ShadowRoot.cpp:

(WebCore::allowsAuthorShadowRoot):

• html/HTMLMediaElement.cpp:

(WebCore::HTMLMediaElement::createShadowSubtree):
(WebCore):
(WebCore::HTMLMediaElement::willAddAuthorShadowRoot):
(WebCore::HTMLMediaElement::createMediaControls):

• html/HTMLMediaElement.h:

(HTMLMediaElement):

LayoutTests:

Adding Shadow DOM to a video element and an audio element is allowed now.

• fast/dom/shadow/shadow-disable-expected.txt:
• fast/dom/shadow/shadow-disable.html:
• fast/dom/shadow/shadowdom-for-media-expected.txt: Added.
• fast/dom/shadow/shadowdom-for-media.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/shadow/shadow-disable-expected.txt (modified) (2 diffs)
• LayoutTests/fast/dom/shadow/shadow-disable.html (modified) (2 diffs)
• LayoutTests/fast/dom/shadow/shadowdom-for-media-expected.txt (added)
• LayoutTests/fast/dom/shadow/shadowdom-for-media.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/ShadowRoot.cpp (modified) (1 diff)
• Source/WebCore/html/HTMLMediaElement.cpp (modified) (2 diffs)
• Source/WebCore/html/HTMLMediaElement.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-07-17  Shinya Kawanaka  <shinyak@chromium.org>
+
+        HTMLMediaElement should not use Element::ensureShadowRoot()
+        https://bugs.webkit.org/show_bug.cgi?id=77936
+
+        Reviewed by Hajime Morita.
+
+        Adding Shadow DOM to a video element and an audio element is allowed now.
+
+        * fast/dom/shadow/shadow-disable-expected.txt:
+        * fast/dom/shadow/shadow-disable.html:
+        * fast/dom/shadow/shadowdom-for-media-expected.txt: Added.
+        * fast/dom/shadow/shadowdom-for-media.html: Added.
+
 2012-07-17  Yoshifumi Inoue  <yosin@chromium.org>
 

trunk/LayoutTests/fast/dom/shadow/shadow-disable-expected.txt

@@ -12 +12 @@
 SECTION
 PASS new WebKitShadowRoot(element) is not null
+AUDIO
+PASS new WebKitShadowRoot(element) is not null
+VIDEO
+PASS new WebKitShadowRoot(element) is not null
 INPUT
 PASS new WebKitShadowRoot(element) threw exception Error: HIERARCHY_REQUEST_ERR: DOM Exception 3.
@@ -17 +21 @@
 PASS new WebKitShadowRoot(element) threw exception Error: HIERARCHY_REQUEST_ERR: DOM Exception 3.
 SELECT
-PASS new WebKitShadowRoot(element) threw exception Error: HIERARCHY_REQUEST_ERR: DOM Exception 3.
-AUDIO
-PASS new WebKitShadowRoot(element) threw exception Error: HIERARCHY_REQUEST_ERR: DOM Exception 3.
-VIDEO
 PASS new WebKitShadowRoot(element) threw exception Error: HIERARCHY_REQUEST_ERR: DOM Exception 3.
 tref

trunk/LayoutTests/fast/dom/shadow/shadow-disable.html

@@ -24 +24 @@
     document.createElement('span'),
     document.createElement('a'),
-    document.createElement('section')
+    document.createElement('section'),
+    document.createElement('audio'),
+    document.createElement('video')
 ];
 
@@ -31 +33 @@
     document.createElement('textarea'),
     document.createElement('select'),
-    document.createElement('audio'),
-    document.createElement('video'),
     document.getElementById('tref')
 ];

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-07-17  Shinya Kawanaka  <shinyak@chromium.org>
+
+        HTMLMediaElement should not use Element::ensureShadowRoot()
+        https://bugs.webkit.org/show_bug.cgi?id=77936
+
+        Reviewed by Hajime Morita.
+
+        a video element and an audio element add UserAgentShadowRoot dynamically, and they assume that it's the oldest ShadowRoot.
+        However an AuthorShadowRoot could be added by a user before a video element and an audio element add UserAgentShadowRoot.
+        It breaks the assumption that the UserAgentShadowRoot is the oldest.
+
+        If the UserAgentShadowRoot is not the oldest, the AuthorShadowRoot a page author added might be ignored.
+        Since the timing to add UserAgentShadowRoot is not known by a user, the fact that UserAgentShadorRoot is
+        not the oldest will cause inconsistent behavior.
+
+        Adding AuthorShadowRoot to a video element and an audio element is allowed by this patch.
+
+        Test: fast/dom/shadow/shadowdom-for-media.html
+
+        * dom/ShadowRoot.cpp:
+        (WebCore::allowsAuthorShadowRoot):
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::createShadowSubtree):
+        (WebCore):
+        (WebCore::HTMLMediaElement::willAddAuthorShadowRoot):
+        (WebCore::HTMLMediaElement::createMediaControls):
+        * html/HTMLMediaElement.h:
+        (HTMLMediaElement):
+
 2012-07-16  Daniel Bates  <dbates@webkit.org>
 

trunk/Source/WebCore/dom/ShadowRoot.cpp

@@ -76 +76 @@
 static bool allowsAuthorShadowRoot(Element* element)
 {
-    // FIXME: MEDIA recreates shadow root dynamically.
-    // https://bugs.webkit.org/show_bug.cgi?id=77936
-    if (element->hasTagName(HTMLNames::videoTag) || element->hasTagName(HTMLNames::audioTag))
-        return false;
-
     // FIXME: ValidationMessage recreates shadow root dynamically.
     // https://bugs.webkit.org/show_bug.cgi?id=77937

trunk/Source/WebCore/html/HTMLMediaElement.cpp

@@ -1861 +1861 @@
 }
 
+void HTMLMediaElement::createShadowSubtree()
+{
+    ASSERT(!shadow() || !shadow()->oldestShadowRoot());
+
+    ShadowRoot::create(this, ShadowRoot::UserAgentShadowRoot);
+}
+
+void HTMLMediaElement::willAddAuthorShadowRoot()
+{
+    ASSERT(shadow());
+    if (shadow()->oldestShadowRoot()) {
+        ASSERT(shadow()->oldestShadowRoot()->type() == ShadowRoot::UserAgentShadowRoot);
+        return;
+    }
+
+    createShadowSubtree();
+}
+
 void HTMLMediaElement::rewind(float timeDelta)
 {
@@ -4191 +4209 @@
         controls->enteredFullscreen();
 
-    ensureShadowRoot()->appendChild(controls, ec);
+    if (!shadow())
+        createShadowSubtree();
+
+    ASSERT(shadow()->oldestShadowRoot()->type() == ShadowRoot::UserAgentShadowRoot);
+    shadow()->oldestShadowRoot()->appendChild(controls, ec);
     return true;
 }

trunk/Source/WebCore/html/HTMLMediaElement.h

@@ -84 +84 @@
 public:
     MediaPlayer* player() const { return m_player.get(); }
-    
+
+    void createShadowSubtree();
+    virtual void willAddAuthorShadowRoot() OVERRIDE;
+
     virtual bool isVideo() const = 0;
     virtual bool hasVideo() const { return false; }