TextTrackLoader should use FetchOptions::mode according its crossOrigin attribute
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Sep 2016 17:24:58 +0000 (17:24 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Sep 2016 17:24:58 +0000 (17:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=161792

Patch by Youenn Fablet <youenn@apple.com> on 2016-09-09
Reviewed by Eric Carlson.

LayoutTests/imported/w3c:

Rebaseline W3C test now that more checks are passing.

* web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrack/addCue-expected.txt:
* web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrack/removeCue-expected.txt:
* web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/endTime-expected.txt:
* web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/id-expected.txt:
* web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/pauseOnExit-expected.txt:
* web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/startTime-expected.txt:
* web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/track-expected.txt:

Source/WebCore:

Covered by existing and updated tests.

Updating text track loader to use fetch mode according crossOrigin value.

Removed the check done in the case the crossOrigin value is not set.
Previously cross-origin loads were forbidden, now this is authorized.
This change allows aligning with the spec.
Also, this check could be bypassed in the case of a same-origin URL redirecting to a cross-origin one.

* loader/TextTrackLoader.cpp:
(WebCore::TextTrackLoader::notifyFinished): Checking resource error in lieu of doing CORS checks on its own.
(WebCore::TextTrackLoader::load): Using CachedResourceRequest::setAsPotentiallyCrossOrigin
* loader/TextTrackLoader.h:
* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::loadFrom): Setting loading and status values as would be done when load is finished.
(WebCore::CachedResource::setBodyDataFrom): Default implementation is to copy the shared buffer.
* loader/cache/CachedResource.h:
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::updateCachedResourceWithCurrentRequest): Enabling resource update when mode or origin is different for TextTrack resources.

LayoutTests:

* http/tests/security/text-track-crossorigin-expected.txt:
* http/tests/security/text-track-crossorigin.html: Updating test to be more robust against timeout.
Updated test to succeed doing no-cors loading of cross-origin resources.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@205750 268f45cc-cd09-0410-ab3c-d52691b4dbfc

17 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/security/text-track-crossorigin-expected.txt
LayoutTests/http/tests/security/text-track-crossorigin.html
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrack/addCue-expected.txt
LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrack/removeCue-expected.txt
LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/endTime-expected.txt
LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/id-expected.txt
LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/pauseOnExit-expected.txt
LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/startTime-expected.txt
LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/track-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/loader/TextTrackLoader.cpp
Source/WebCore/loader/TextTrackLoader.h
Source/WebCore/loader/cache/CachedResource.cpp
Source/WebCore/loader/cache/CachedResource.h
Source/WebCore/loader/cache/CachedResourceLoader.cpp

index 160270a..268c0e7 100644 (file)
@@ -1,3 +1,14 @@
+2016-09-09  Youenn Fablet  <youenn@apple.com>
+
+        TextTrackLoader should use FetchOptions::mode according its crossOrigin attribute
+        https://bugs.webkit.org/show_bug.cgi?id=161792
+
+        Reviewed by Eric Carlson.
+
+        * http/tests/security/text-track-crossorigin-expected.txt:
+        * http/tests/security/text-track-crossorigin.html: Updating test to be more robust against timeout.
+        Updated test to succeed doing no-cors loading of cross-origin resources.
+
 2016-09-09  Ryan Haddad  <ryanhaddad@apple.com>
 
         Marking imported/w3c/web-platform-tests/IndexedDB/keyorder.htm as flaky on mac-wk2 debug.
index 3c565ae..93226c6 100644 (file)
@@ -1,13 +1,13 @@
-CONSOLE MESSAGE: Cross-origin text track load denied by Cross-Origin Resource Sharing policy.
+CONSOLE MESSAGE: Origin http://127.0.0.1:8000 is not allowed by Access-Control-Allow-Origin.
 CONSOLE MESSAGE: Cross-origin text track load denied by Cross-Origin Resource Sharing policy.
 Tests loading cross-domain <track>.
 
 
 Loading without Access-Control-Allow-Origin header, no "crossorigin" attribute on <video>
-EVENT(error)
-PASS: shouldLoad should be 'false' and is.
+EVENT(load)
+PASS: shouldLoad should be 'true' and is.
 PASS: event.target should be '[object HTMLTrackElement]' and is.
-PASS: trackElement.readyState should be '3' and is.
+PASS: trackElement.readyState should be '2' and is.
 
 
 Loading without Access-Control-Allow-Origin header, setting video.crossorigin to "anonymous"
index f9d7f02..269c7d5 100644 (file)
@@ -4,7 +4,7 @@
         <script src="resources/cross-frame-access.js"></script>
         <script>
 
-            var shouldLoad = false;
+            var shouldLoad = true;
             var counter = 0;
 
             if (window.testRunner) {
 
                 log('<br>');
                 switch(counter) {
+                case 0:
+                    log('Loading <b>without</b> Access-Control-Allow-Origin header, setting video.crossorigin to "anonymous"');
+                    url = "http://localhost:8000/security/resources/captions-with-access-control-headers.php?count=" + counter;
+                    videoElement.setAttribute('crossorigin', 'anonymous');
+                    trackElement.removeAttribute('src');
+                    trackElement.setAttribute('src', url);
+                    shouldLoad = false;
+                    ++counter;
+                    break;
+
                 case 2:
                     log('Loading <b>with</b> Access-Control-Allow-Origin and Access-Control-Allow-Credentials headers, setting video.crossorigin to "use-credentials"');
                     url = "http://localhost:8000/security/resources/captions-with-access-control-headers.php?origin=1;credentials=1";
@@ -41,6 +51,9 @@
                     log("END OF TEST");
                     if (window.testRunner)
                         testRunner.notifyDone();
+                defaut:
+                    if (window.testRunner)
+                        testRunner.notifyDone();
                 }
             }
 
 
                 log('<br>');
                 switch(counter) {
-                case 0:
-                    log('Loading <b>without</b> Access-Control-Allow-Origin header, setting video.crossorigin to "anonymous"');
-                    url = "http://localhost:8000/security/resources/captions-with-access-control-headers.php?count=" + counter;
-                    videoElement.setAttribute('crossorigin', 'anonymous');
-                    trackElement.removeAttribute('src');
-                    trackElement.setAttribute('src', url);
-                    ++counter;
-                    break;
-
                 case 1:
                     log('Loading <b>with</b> Access-Control-Allow-Origin header, leaving video.crossorigin as "anonymous"');
                     url = "http://localhost:8000/security/resources/captions-with-access-control-headers.php?origin=1";
                     shouldLoad = true;
                     ++counter;
                     break;
+                defaut:
+                    if (window.testRunner)
+                        testRunner.notifyDone();
                 }
-
             }
 
             function start()
index 7f8ec3d..a5967c4 100644 (file)
@@ -1,5 +1,22 @@
 2016-09-09  Youenn Fablet  <youenn@apple.com>
 
+        TextTrackLoader should use FetchOptions::mode according its crossOrigin attribute
+        https://bugs.webkit.org/show_bug.cgi?id=161792
+
+        Reviewed by Eric Carlson.
+
+        Rebaseline W3C test now that more checks are passing.
+
+        * web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrack/addCue-expected.txt:
+        * web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrack/removeCue-expected.txt:
+        * web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/endTime-expected.txt:
+        * web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/id-expected.txt:
+        * web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/pauseOnExit-expected.txt:
+        * web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/startTime-expected.txt:
+        * web-platform-tests/html/semantics/embedded-content/media-elements/interfaces/TextTrackCue/track-expected.txt:
+
+2016-09-09  Youenn Fablet  <youenn@apple.com>
+
         Sync web-platform-tests up to revision 6d9c836
         https://bugs.webkit.org/show_bug.cgi?id=161738
 
index 7a7cf59..b517d32 100644 (file)
@@ -1,8 +1,7 @@
-CONSOLE MESSAGE: Cross-origin text track load denied by Cross-Origin Resource Sharing policy.
 
 PASS TextTrack.addCue(), adding a cue to two different tracks 
 PASS TextTrack.addCue(), adding a cue to a track twice 
 PASS TextTrack.addCue(), adding a removed cue to a different track 
 PASS TextTrack.addCue(), adding an associated but removed cue to the same track 
-FAIL TextTrack.addCue(), adding a cue associated with a track element to other track assert_unreached: got error event Reached unreachable code
+PASS TextTrack.addCue(), adding a cue associated with a track element to other track 
 
index d28fd88..9078317 100644 (file)
@@ -1,5 +1,4 @@
-CONSOLE MESSAGE: Cross-origin text track load denied by Cross-Origin Resource Sharing policy.
 
 PASS TextTrack.removeCue(), two elementless tracks 
-FAIL TextTrack.removeCue(), cue from track element assert_unreached: got error event Reached unreachable code
+PASS TextTrack.removeCue(), cue from track element 
 
index 6e152cf..df94e8f 100644 (file)
@@ -1,5 +1,4 @@
-CONSOLE MESSAGE: Cross-origin text track load denied by Cross-Origin Resource Sharing policy.
 
 PASS TextTrackCue.endTime, script-created cue 
-FAIL TextTrackCue.endTime, parsed cue assert_unreached: got error event Reached unreachable code
+FAIL TextTrackCue.endTime, parsed cue null is not an object (evaluating 'assert_equals')
 
index e5b7265..cdd5d71 100644 (file)
@@ -1,5 +1,4 @@
-CONSOLE MESSAGE: Cross-origin text track load denied by Cross-Origin Resource Sharing policy.
 
 PASS TextTrackCue.id, script-created cue 
-FAIL TextTrackCue.id, parsed cue assert_unreached: got error event Reached unreachable code
+FAIL TextTrackCue.id, parsed cue null is not an object (evaluating 'assert_equals')
 
index 43a3725..f16daa7 100644 (file)
@@ -1,5 +1,4 @@
-CONSOLE MESSAGE: Cross-origin text track load denied by Cross-Origin Resource Sharing policy.
 
 PASS TextTrackCue.pauseOnExit, script-created cue 
-FAIL TextTrackCue.pauseOnExit, parsed cue assert_unreached: got error event Reached unreachable code
+FAIL TextTrackCue.pauseOnExit, parsed cue null is not an object (evaluating 't.track.cues')
 
index 6f16ca7..ee4706d 100644 (file)
@@ -1,5 +1,4 @@
-CONSOLE MESSAGE: Cross-origin text track load denied by Cross-Origin Resource Sharing policy.
 
 PASS TextTrackCue.startTime, script-created cue 
-FAIL TextTrackCue.startTime, parsed cue assert_unreached: got error event Reached unreachable code
+FAIL TextTrackCue.startTime, parsed cue null is not an object (evaluating 'assert_equals')
 
index 996c257..0a81c10 100644 (file)
@@ -1,5 +1,4 @@
-CONSOLE MESSAGE: Cross-origin text track load denied by Cross-Origin Resource Sharing policy.
 
 PASS TextTrackCue.track, script-created cue 
-FAIL TextTrackCue.track, parsed cue assert_unreached: got error event Reached unreachable code
+FAIL TextTrackCue.track, parsed cue null is not an object (evaluating 't.track.cues')
 
index e787361..99a5390 100644 (file)
@@ -1,3 +1,30 @@
+2016-09-09  Youenn Fablet  <youenn@apple.com>
+
+        TextTrackLoader should use FetchOptions::mode according its crossOrigin attribute
+        https://bugs.webkit.org/show_bug.cgi?id=161792
+
+        Reviewed by Eric Carlson.
+
+        Covered by existing and updated tests.
+
+        Updating text track loader to use fetch mode according crossOrigin value.
+
+        Removed the check done in the case the crossOrigin value is not set.
+        Previously cross-origin loads were forbidden, now this is authorized.
+        This change allows aligning with the spec.
+        Also, this check could be bypassed in the case of a same-origin URL redirecting to a cross-origin one.
+
+        * loader/TextTrackLoader.cpp:
+        (WebCore::TextTrackLoader::notifyFinished): Checking resource error in lieu of doing CORS checks on its own.
+        (WebCore::TextTrackLoader::load): Using CachedResourceRequest::setAsPotentiallyCrossOrigin
+        * loader/TextTrackLoader.h:
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::loadFrom): Setting loading and status values as would be done when load is finished.
+        (WebCore::CachedResource::setBodyDataFrom): Default implementation is to copy the shared buffer.
+        * loader/cache/CachedResource.h:
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::updateCachedResourceWithCurrentRequest): Enabling resource update when mode or origin is different for TextTrack resources.
+
 2016-09-09  Alex Christensen  <achristensen@webkit.org>
 
         URLParser should parse URLs with non-special schemes
index c8010c6..0675adc 100644 (file)
@@ -124,8 +124,7 @@ void TextTrackLoader::notifyFinished(CachedResource* resource)
 {
     ASSERT(m_resource == resource);
 
-    Document* document = downcast<Document>(m_scriptExecutionContext);
-    if (!m_crossOriginMode.isNull() && !resource->passesSameOriginPolicyCheck(*document->securityOrigin()))
+    if (resource->resourceError().isAccessControl())
         corsPolicyPreventedLoad();
 
     if (m_state != Failed) {
@@ -141,7 +140,7 @@ void TextTrackLoader::notifyFinished(CachedResource* resource)
 
     if (!m_cueLoadTimer.isActive())
         m_cueLoadTimer.startOneShot(0);
-    
+
     cancelLoad();
 }
 
@@ -156,25 +155,14 @@ bool TextTrackLoader::load(const URL& url, const String& crossOriginMode, bool i
     options.contentSecurityPolicyImposition = isInitiatingElementInUserAgentShadowTree ? ContentSecurityPolicyImposition::SkipPolicyCheck : ContentSecurityPolicyImposition::DoPolicyCheck;
 
     CachedResourceRequest cueRequest(ResourceRequest(document->completeURL(url)), options);
-
-    if (!crossOriginMode.isNull()) {
-        m_crossOriginMode = crossOriginMode;
-        StoredCredentials allowCredentials = equalLettersIgnoringASCIICase(crossOriginMode, "use-credentials") ? AllowStoredCredentials : DoNotAllowStoredCredentials;
-        updateRequestForAccessControl(cueRequest.mutableResourceRequest(), *document->securityOrigin(), allowCredentials);
-    } else {
-        // Cross-origin resources that are not suitably CORS-enabled may not load.
-        if (!document->securityOrigin()->canRequest(url)) {
-            corsPolicyPreventedLoad();
-            return false;
-        }
-    }
+    cueRequest.setAsPotentiallyCrossOrigin(crossOriginMode, *document);
 
     m_resource = document->cachedResourceLoader().requestTextTrack(cueRequest);
     if (!m_resource)
         return false;
 
     m_resource->addClient(this);
-    
+
     return true;
 }
 
index 49ed39f..123f148 100644 (file)
@@ -83,7 +83,6 @@ private:
     CachedResourceHandle<CachedTextTrack> m_resource;
     ScriptExecutionContext* m_scriptExecutionContext;
     Timer m_cueLoadTimer;
-    String m_crossOriginMode;
     State m_state;
     unsigned m_parseOffset;
     bool m_newCuesAvailable;
index f13ae62..db7bf2e 100644 (file)
@@ -358,6 +358,13 @@ void CachedResource::loadFrom(const CachedResource& resource, const ResourceLoad
     }
 
     setBodyDataFrom(resource);
+    setStatus(Status::Cached);
+    setLoading(false);
+}
+
+void CachedResource::setBodyDataFrom(const CachedResource& resource)
+{
+    m_data = resource.m_data;
 }
 
 void CachedResource::checkNotify()
index 54b957d..4e531b3 100644 (file)
@@ -307,7 +307,7 @@ private:
 
     virtual void checkNotify();
     virtual bool mayTryReplaceEncodedData() const { return false; }
-    virtual void setBodyDataFrom(const CachedResource&) { }
+    virtual void setBodyDataFrom(const CachedResource&);
 
     std::chrono::microseconds freshnessLifetime(const ResourceResponse&) const;
 
index d5b7f8c..9a7b2fd 100644 (file)
@@ -546,7 +546,7 @@ bool CachedResourceLoader::updateCachedResourceWithCurrentRequest(CachedResource
     CachedResource& resource = *resourceHandle;
 
     // FIXME: We should progressively extend this to other reusable resources
-    if (resource.type() != CachedResource::Type::ImageResource)
+    if (resource.type() != CachedResource::Type::ImageResource && resource.type() != CachedResource::Type::TextTrackResource)
         return false;
 
     bool shouldUpdate = resource.options().mode != request.options().mode || request.resourceRequest().httpOrigin() != resource.resourceRequest().httpOrigin();