Bug 129687 - [Mac][WK2] Videos do not have access to session cookies
Summary: [Mac][WK2] Videos do not have access to session cookies
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-04 10:52 PST by Jer Noble
Modified: 2014-04-08 11:27 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.53 KB, patch)
2014-03-04 10:55 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (3.55 KB, patch)
2014-03-10 10:14 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (3.77 KB, patch)
2014-03-31 17:10 PDT, Jer Noble
eric.carlson: review+
Details | Formatted Diff | Diff
Patch for landing (3.82 KB, patch)
2014-04-01 12:54 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (3.96 KB, patch)
2014-04-04 14:03 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2014-03-04 10:52:34 PST
[Mac][WK2] Videos do not have access to session cookies
Comment 1 Jer Noble 2014-03-04 10:55:46 PST
Created attachment 225792 [details]
Patch
Comment 2 Eric Carlson 2014-03-04 18:12:55 PST
Comment on attachment 225792 [details]
Patch

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

> Source/WebKit2/Shared/mac/CookieStorageShim.mm:36
> +#include <Foundation/NSURLSession.h>

This needs to be conditionally included.

> Source/WebKit2/Shared/mac/CookieStorageShim.mm:99
> +- (void)_getCookieHeadersForTask:(NSURLSessionTask*)task completionHandler:(void (^)(CFDictionaryRef))completionHandler

Can completionHandler be NULL?
Comment 3 Jer Noble 2014-03-10 10:14:43 PDT
Created attachment 226315 [details]
Patch for landing
Comment 4 Jer Noble 2014-03-31 17:10:58 PDT
Created attachment 228221 [details]
Patch
Comment 5 Eric Carlson 2014-04-01 09:10:48 PDT
Comment on attachment 228221 [details]
Patch

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

> Source/WebKit2/Shared/mac/CookieStorageShim.mm:116
> +    dispatch_async(dispatch_get_main_queue(), ^{
> +        completionHandler(WebKit::webKitCookieStorageCopyRequestHeaderFieldsForURL(nullptr, (CFURLRef)[[task currentRequest] URL]));

Can completionHandler be NULL?
Comment 6 Jer Noble 2014-04-01 12:54:23 PDT
Created attachment 228309 [details]
Patch for landing
Comment 7 Jer Noble 2014-04-04 14:03:02 PDT
Created attachment 228615 [details]
Patch for landing
Comment 8 Alexey Proskuryakov 2014-04-04 15:41:21 PDT
Comment on attachment 228221 [details]
Patch

Obsoleting an old version of the patch.
Comment 9 WebKit Commit Bot 2014-04-04 16:15:37 PDT
Comment on attachment 228615 [details]
Patch for landing

Clearing flags on attachment: 228615

Committed r166812: <http://trac.webkit.org/changeset/166812>
Comment 10 WebKit Commit Bot 2014-04-04 16:15:40 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Joseph Pecoraro 2014-04-04 18:18:52 PDT
(In reply to comment #10)
> All reviewed patches have been landed.  Closing bug.

Looks like it this broke 32-bit builds on Mavericks.

    CompileC CookieStorageShim.o
    OpenSource/Source/WebKit2/Shared/mac/CookieStorageShim.mm:54:35: error: expected a type
    - (void)_getCookieHeadersForTask:(NSURLSessionTask*)task completionHandler:(void (^)(CFDictionaryRef))completionHandler;
                                      ^
    OpenSource/Source/WebKit2/Shared/mac/CookieStorageShim.mm:117:35: error: expected a type
    - (void)_getCookieHeadersForTask:(NSURLSessionTask*)task completionHandler:(void (^)(CFDictionaryRef))completionHandler
                                      ^
    2 errors generated.

I'm going to roll it out, because if that is the case then this bug probably needs to be addressed differently / tested on 32 bit.
Comment 12 Joseph Pecoraro 2014-04-04 18:25:20 PDT
Rolled out in: <http://trac.webkit.org/changeset/166819>
Comment 13 Jer Noble 2014-04-07 10:50:37 PDT
(In reply to comment #12)
> Rolled out in: <http://trac.webkit.org/changeset/166819>

Just needed a forward declaration of NSURLSessionTask.
Comment 14 Jer Noble 2014-04-08 11:27:18 PDT
Committed r166940: <http://trac.webkit.org/changeset/166940>