Bug 129687

Summary: [Mac][WK2] Videos do not have access to session cookies
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, jer.noble, joepeck
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing
none
Patch
eric.carlson: review+
Patch for landing
none
Patch for landing none

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>