Bug 145650

Summary: [Web Timing] Use new SPI to enable data collection.
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: WebCore Misc.Assignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, bfulgham, cdumez, cmarcelo, commit-queue, dbates, ddkilzer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch cdumez: review+

Description Alex Christensen 2015-06-04 12:00:22 PDT
There is a new SPI.  Let's use it.
Comment 1 Alex Christensen 2015-06-04 12:02:13 PDT
Created attachment 254284 [details]
Patch
Comment 2 Alex Christensen 2015-06-04 12:03:09 PDT
This patch does not actually enable data collection.  Someone who knows the SPI should point out what I'm doing wrong.  Maybe it's a strange ObjC problem.  Maybe I'm using the SPI wrong.
Comment 3 Simon Fraser (smfr) 2015-06-04 14:39:58 PDT
Comment on attachment 254284 [details]
Patch

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

> Source/WebCore/platform/network/mac/ResourceHandleMac.mm:74
> +// FIXME: Move this to CFNetworkSPI.h.

Seems like this would be a good time to do that.
Comment 4 Alex Christensen 2015-06-04 15:39:10 PDT
Created attachment 254308 [details]
Patch
Comment 5 Alex Christensen 2015-06-04 17:01:20 PDT
Created attachment 254316 [details]
Patch
Comment 6 Alex Christensen 2015-06-04 17:40:22 PDT
(In reply to comment #3)
> Seems like this would be a good time to do that.
I disagree. This is already cross-platform and platform dependent enough. I don't want to mess with breaking Windows in this patch.
Comment 7 Alex Christensen 2015-06-04 17:46:53 PDT
Created attachment 254321 [details]
Patch
Comment 8 Chris Dumez 2015-06-05 11:19:08 PDT
Comment on attachment 254321 [details]
Patch

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

> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:65
> +#if PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 90000

Shouldn't this be in Platform.h ?

> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:215
> +#if HAVE_TIMINGDATAOPTIONS

#if HAVE(TIMINGDATAOPTIONS)

> Source/WebCore/platform/network/mac/ResourceHandleMac.mm:68
> +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100) || (TARGET_OS_IPHONE && __IPHONE_OS_VERSION_MIN_REQUIRED >= 90000)

Shouldn't this be in Platform.h?

> Source/WebCore/platform/network/mac/ResourceHandleMac.mm:79
> +#if !HAVE_TIMINGDATAOPTIONS

#if !HAVE(TIMINGDATAOPTIONS)

> Source/WebCore/platform/network/mac/ResourceHandleMac.mm:84
> +#if HAVE_TIMINGDATAOPTIONS

#if HAVE(TIMINGDATAOPTIONS)

> Source/WebCore/platform/network/mac/ResourceHandleMac.mm:236
> +#if HAVE_TIMINGDATAOPTIONS

#if HAVE(TIMINGDATAOPTIONS)

> Source/WebCore/platform/network/mac/ResourceHandleMac.mm:765
> +#if !HAVE_TIMINGDATAOPTIONS

#if !HAVE(TIMINGDATAOPTIONS)
Comment 9 Alex Christensen 2015-06-05 13:30:09 PDT
Comment on attachment 254321 [details]
Patch

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

> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:218
> +    CFDictionaryAddValue(propertiesDictionary.get(), CFSTR("kCFURLConnectionURLConnectionProperties"), &TimingDataOptionsEnableW3CNavigationTiming);

timingDataOptionsDictionary here, not propertiesDictionary.
Comment 10 Alex Christensen 2015-06-05 14:00:09 PDT
Comment on attachment 254321 [details]
Patch

And I need to call CFNumberCreate on iOS.
Comment 11 Alex Christensen 2015-06-05 14:12:22 PDT
Created attachment 254381 [details]
Patch
Comment 12 Chris Dumez 2015-06-05 14:26:18 PDT
Comment on attachment 254381 [details]
Patch

r=me
Comment 13 Alex Christensen 2015-06-05 14:38:38 PDT
http://trac.webkit.org/changeset/185264
Comment 14 Brent Fulgham 2015-06-25 16:08:47 PDT
<rdar://problem/21203358>