Bug 145650 - [Web Timing] Use new SPI to enable data collection.
Summary: [Web Timing] Use new SPI to enable data collection.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-06-04 12:00 PDT by Alex Christensen
Modified: 2015-06-25 16:08 PDT (History)
7 users (show)

See Also:

Attachments
Patch (3.09 KB, patch)
2015-06-04 12:02 PDT, Alex Christensen 		no flags Details | Formatted Diff | Diff
Patch (3.12 KB, patch)
2015-06-04 15:39 PDT, Alex Christensen 		no flags Details | Formatted Diff | Diff
Patch (4.98 KB, patch)
2015-06-04 17:01 PDT, Alex Christensen 		no flags Details | Formatted Diff | Diff
Patch (5.02 KB, patch)
2015-06-04 17:46 PDT, Alex Christensen 		no flags Details | Formatted Diff | Diff
Patch (5.61 KB, patch)
2015-06-05 14:12 PDT, Alex Christensen 		cdumez: review+
Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>