Bug 120532 - Fix SynchronousLoaderClient class for !USE(CFNETWORK) platforms
Summary: Fix SynchronousLoaderClient class for !USE(CFNETWORK) platforms
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: Csaba Osztrogonác
URL:
Keywords:
: 120533 (view as bug list)
Depends on: 120539
Blocks: 108832
  Show dependency treegraph
 
Reported: 2013-08-30 09:32 PDT by Csaba Osztrogonác
Modified: 2013-09-05 02:50 PDT (History)
10 users (show)

See Also:


Attachments
Patch (1.71 KB, patch)
2013-08-30 09:33 PDT, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
patch-for-ews (1.67 KB, patch)
2013-08-30 10:56 PDT, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
patch-for-ews (11.25 KB, patch)
2013-09-02 02:39 PDT, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Patch (11.28 KB, patch)
2013-09-04 14:01 PDT, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2013-08-30 09:32:07 PDT
SynchronousLoaderClient::didReceiveAuthenticationChallengedidReceiveAuthenticationChallenge()
is guarded by USE(CFNETWORK) in Source/WebCore/platform/network/SynchronousLoaderClient.cpp,
but isn't in its header.

It causes link failure on !CF platforms with enabled NetworkProcess. Guarding this
function is safe, because it is implemented in the base class of SynchronousLoaderClient
by ResourceHandleClient. (It is an empty function.)
Comment 1 Csaba Osztrogonác 2013-08-30 09:33:47 PDT
Created attachment 210121 [details]
Patch
Comment 2 Darin Adler 2013-08-30 09:36:20 PDT
Comment on attachment 210121 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        No new tests (OOPS!).

Commit queue can’t handle a patch that has a line like this in it.
Comment 3 Csaba Osztrogonác 2013-08-30 09:45:08 PDT
Committed r154891: <http://trac.webkit.org/changeset/154891>
Comment 4 Jessie Berlin 2013-08-30 10:32:51 PDT
This broke the Mac build

http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20%28Build%29/builds/15596/steps/compile-webkit/logs/stdio

/Volumes/Data/slave/mountainlion-debug/build/Source/WebCore/platform/network/mac/SynchronousLoaderClient.mm:35:31: error: out-of-line definition of 'didReceiveAuthenticationChallenge' does not match any declaration in 'WebCore::SynchronousLoaderClient'
void SynchronousLoaderClient::didReceiveAuthenticationChallenge(ResourceHandle*, const AuthenticationChallenge& challenge)
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
Comment 5 WebKit Commit Bot 2013-08-30 10:38:25 PDT
Re-opened since this is blocked by bug 120539
Comment 6 Csaba Osztrogonác 2013-08-30 10:44:00 PDT
Sorry for the breakage, I rolled it out for now, will fix a little bit later.
Comment 7 Csaba Osztrogonác 2013-08-30 10:56:08 PDT
Created attachment 210133 [details]
patch-for-ews

patch for EWS
Comment 8 Csaba Osztrogonác 2013-09-02 02:28:44 PDT
*** Bug 120533 has been marked as a duplicate of this bug. ***
Comment 9 Csaba Osztrogonác 2013-09-02 02:39:39 PDT
Created attachment 210271 [details]
patch-for-ews

CFNETWORK implementation of SynchronousLoaderClient::didReceiveAuthenticationChallenge() and SynchronousLoaderClient::platformBadResponseError() shouldn't be in the cross-platform SynchronousLoaderClient.cpp, but in cf/SynchronousLoaderClientCFNet.cpp
Comment 10 Csaba Osztrogonác 2013-09-02 08:16:44 PDT
Comment on attachment 210271 [details]
patch-for-ews

EWS bots are happy, so r? for this patch.
Comment 11 Csaba Osztrogonác 2013-09-03 14:56:10 PDT
Ping
Comment 12 Darin Adler 2013-09-04 09:27:43 PDT
Comment on attachment 210271 [details]
patch-for-ews

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

> Source/WebCore/platform/network/cf/SynchronousLoaderClientCFNet.cpp:34
> +#if USE(CFNETWORK)
> +#include <CFNetwork/CFURLConnectionPriv.h>
> +#endif

If this entire file is only for CFNetwork, then why the #if? If we do have an #if, it should wrap the whole file, not just the functions and this one include.
Comment 13 Csaba Osztrogonác 2013-09-04 14:01:47 PDT
Created attachment 210492 [details]
Patch

I used #if guards similar to other *CFNet.cpp. But you're right, guarding the whole file is reasonable, so I fixed it.
Comment 14 Csaba Osztrogonác 2013-09-05 02:50:26 PDT
Comment on attachment 210492 [details]
Patch

Clearing flags on attachment: 210492

Committed r155108: <http://trac.webkit.org/changeset/155108>
Comment 15 Csaba Osztrogonác 2013-09-05 02:50:34 PDT
All reviewed patches have been landed.  Closing bug.