Bug 133445 - [iOS] WebCore fails to build: platform/network/mac/ResourceHandleMac.mm:729:62: error: use of undeclared identifier '_CFURLConnectionCopyTimingData'
Summary: [iOS] WebCore fails to build: platform/network/mac/ResourceHandleMac.mm:729:6...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-06-02 07:06 PDT by David Kilzer (:ddkilzer)
Modified: 2014-06-02 17:37 PDT (History)
4 users (show)

See Also:


Attachments
Patch v1 (1.60 KB, patch)
2014-06-02 07:23 PDT, David Kilzer (:ddkilzer)
ap: review+
ap: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2014-06-02 07:06:06 PDT
Seeing these errors on some internal buildbots:

Source/WebCore/platform/network/mac/ResourceHandleMac.mm:729:62: error: use of undeclared identifier '_CFURLConnectionCopyTimingData'
Source/WebCore/platform/network/mac/ResourceHandleMac.mm:727:97: error: unused parameter 'timing' [-Werror,-Wunused-parameter]
void ResourceHandle::getConnectionTimingData(CFURLConnectionRef connection, ResourceLoadTiming& timing)
2 errors generated.
Comment 1 David Kilzer (:ddkilzer) 2014-06-02 07:06:30 PDT
<rdar://problem/17090035>
Comment 2 David Kilzer (:ddkilzer) 2014-06-02 07:23:53 PDT
Created attachment 232377 [details]
Patch v1
Comment 3 David Kilzer (:ddkilzer) 2014-06-02 09:04:10 PDT
This patch built with an iOS Release build internally as well.
Comment 4 Alexey Proskuryakov 2014-06-02 09:17:08 PDT
Comment on attachment 232377 [details]
Patch v1

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

I don't understand this patch, what are the bots that fail? Everything was building correctly for a long time.

r=meto fix the build anyway.

> Source/WebCore/platform/network/mac/ResourceHandleMac.mm:64
> +#else
> +typedef struct _CFURLConnection* CFURLConnectionRef;
> +extern "C" {
> +CFDictionaryRef _CFURLConnectionCopyTimingData(CFURLConnectionRef);
> +}
> +#endif

This shouldn't be an #else. We want to have the declarations even when the private header is included, because this way, we will find out about changes in SPIs sooner and more reliably. 

It's very difficult to debug issues caused by incorrect SPI declarations.

Also, this should be under #if USE(CFNETWORK), to match the code that uses CFNetwork.
Comment 5 David Kilzer (:ddkilzer) 2014-06-02 16:35:09 PDT
(In reply to comment #4)
> (From update of attachment 232377 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=232377&action=review
> 
> I don't understand this patch, what are the bots that fail? Everything was building correctly for a long time.

See internal email thread, or the radar.

> r=meto fix the build anyway.
> 
> > Source/WebCore/platform/network/mac/ResourceHandleMac.mm:64
> > +#else
> > +typedef struct _CFURLConnection* CFURLConnectionRef;
> > +extern "C" {
> > +CFDictionaryRef _CFURLConnectionCopyTimingData(CFURLConnectionRef);
> > +}
> > +#endif
> 
> This shouldn't be an #else. We want to have the declarations even when the private header is included, because this way, we will find out about changes in SPIs sooner and more reliably. 
> 
> It's very difficult to debug issues caused by incorrect SPI declarations.
> 
> Also, this should be under #if USE(CFNETWORK), to match the code that uses CFNetwork.

I thought we tried to avoid adding headers in #if/#endif blocks unless absolutely necessary.

Technically, both this and the __has_include() below it should be in outer #if ENABLE(WEB_TIMING)/#endif macros as well.  Do we really want to do that?
Comment 6 Alexey Proskuryakov 2014-06-02 17:07:54 PDT
> I thought we tried to avoid adding headers in #if/#endif blocks unless absolutely necessary.

These are not WebKit headers, so I think that it's the right thing to do. With WebKit headers, yes, we try to have the guards inside the headers.
Comment 7 David Kilzer (:ddkilzer) 2014-06-02 17:37:55 PDT
Committed r169551: <http://trac.webkit.org/changeset/169551>