Bug 163263 - Remove dead networking code
Summary: Remove dead networking code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-10 19:14 PDT by Alex Christensen
Modified: 2016-10-11 11:23 PDT (History)
2 users (show)

See Also:


Attachments
Patch (196.05 KB, patch)
2016-10-10 19:20 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (196.22 KB, patch)
2016-10-10 19:24 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (202.07 KB, patch)
2016-10-10 19:34 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (199.78 KB, patch)
2016-10-10 21:10 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (187.61 KB, patch)
2016-10-10 21:26 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (187.73 KB, patch)
2016-10-10 22:52 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (188.04 KB, patch)
2016-10-11 08:24 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (188.04 KB, patch)
2016-10-11 09:40 PDT, Alex Christensen
dbates: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2016-10-10 19:14:56 PDT
Remove dead networking code
Comment 1 Alex Christensen 2016-10-10 19:20:26 PDT
Created attachment 291209 [details]
Patch
Comment 2 WebKit Commit Bot 2016-10-10 19:22:42 PDT
Attachment 291209 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 88 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Alex Christensen 2016-10-10 19:24:09 PDT
Created attachment 291210 [details]
Patch
Comment 4 Alex Christensen 2016-10-10 19:34:10 PDT
Created attachment 291213 [details]
Patch
Comment 5 Alex Christensen 2016-10-10 21:10:57 PDT
Created attachment 291224 [details]
Patch
Comment 6 Alex Christensen 2016-10-10 21:26:22 PDT
Created attachment 291227 [details]
Patch
Comment 7 WebKit Commit Bot 2016-10-10 21:27:55 PDT
Attachment 291227 [details] did not pass style-queue:


ERROR: Source/WebCore/WebCorePrefix.h:0:  Use #pragma once header guard.  [build/header_guard] [5]
ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:0:  Use #pragma once header guard.  [build/header_guard] [5]
Total errors found: 2 in 88 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Alex Christensen 2016-10-10 22:52:15 PDT
Created attachment 291232 [details]
Patch
Comment 9 WebKit Commit Bot 2016-10-10 22:53:38 PDT
Attachment 291232 [details] did not pass style-queue:


ERROR: Source/WebCore/WebCorePrefix.h:0:  Use #pragma once header guard.  [build/header_guard] [5]
ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:0:  Use #pragma once header guard.  [build/header_guard] [5]
Total errors found: 2 in 88 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Darin Adler 2016-10-10 23:53:04 PDT
Comment on attachment 291232 [details]
Patch

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

> Source/WebCore/loader/EmptyClients.h:406
> -#if PLATFORM(WIN) && USE(CFNETWORK)
> +#if PLATFORM(WIN) && USE(CFURLCONNECTION)

Do we need to mention PLATFORM(WIN) here at all now?

> Source/WebCore/loader/FrameLoaderClient.h:300
> -#if PLATFORM(WIN) && USE(CFNETWORK)
> +#if PLATFORM(WIN) && USE(CFURLCONNECTION)

Do we need to mention PLATFORM(WIN) here at all now?

> Source/WebCore/loader/ResourceLoader.cpp:-723
>      // Only these platforms provide a way to continue without credentials.
>      // If we can't continue with credentials, we need to cancel the load altogether.
> -#if PLATFORM(COCOA) || USE(CFNETWORK) || USE(CURL) || PLATFORM(GTK) || PLATFORM(EFL)
>      challenge.authenticationClient()->receivedRequestToContinueWithoutCredential(challenge);
>      ASSERT(!m_handle || !m_handle->hasAuthenticationChallenge());
> -#else
> -    didFail(blockedError());
> -#endif

This comment does not make sense any more if we remove the #if and the #else.

> Source/WebCore/platform/mac/WebCoreSystemInterface.h:83
> -#if USE(CFNETWORK)
> +#if USE(CFURLCONNECTION)
>  typedef struct OpaqueCFHTTPCookieStorage*  CFHTTPCookieStorageRef;
>  typedef struct _CFURLProtectionSpace* CFURLProtectionSpaceRef;
>  typedef struct _CFURLCredential* WKCFURLCredentialRef;

This isn’t used on Windows, so why keep the USE(CFURLCONNECTION) part of this?

> Source/WebCore/platform/network/ProtectionSpaceBase.cpp:-33
> -#if USE(CFNETWORK) && !PLATFORM(COCOA)
> -#include "AuthenticationCF.h"
> -#include <CFNetwork/CFURLProtectionSpacePriv.h>
> -#endif

Why isn’t this needed on Windows?

> Source/WebCore/platform/network/ResourceHandle.h:95
> +class ResourceHandle : public RefCounted<ResourceHandle> , public AuthenticationClient {

No space before the comma, please.

> Source/WebCore/platform/network/ResourceRequestBase.cpp:35
> +#if !USE(SOUP) && (!PLATFORM(COCOA) || USE(CFURLCONNECTION))

Seems like this can just be !USE(SOUP) && !PLATFORM(COCOA), no need to check USE(CFURLCONNECTION)

> Source/WebCore/platform/network/cf/NetworkStorageSessionCFNet.cpp:101
> -#if USE(CFNETWORK)
> +#if USE(CFURLCONNECTION)
>      return _CFHTTPCookieStorageGetDefault(kCFAllocatorDefault);
>  #else
>      // When using NSURLConnection, we also use its shared cookie storage.
> -    return 0;
> +    return nullptr;
>  #endif

Should this file even compile anything when USE(CFURLCONNETION) is false?

> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:-478
> -#if PLATFORM(COCOA)
>              CFURLConnectionUseCredential(d->m_connection.get(), webCredential.cfCredential(), challenge.cfURLAuthChallengeRef());
> -#else
> -            RetainPtr<CFURLCredentialRef> cfCredential = adoptCF(createCF(webCredential));
> -            CFURLConnectionUseCredential(d->m_connection.get(), cfCredential.get(), challenge.cfURLAuthChallengeRef());
> -#endif

Here you kept the Cocoa version. I think that’s backwards.

> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:-486
> -#if PLATFORM(COCOA)
>          CFURLConnectionUseCredential(d->m_connection.get(), credential.cfCredential(), challenge.cfURLAuthChallengeRef());
> -#else
> -        RetainPtr<CFURLCredentialRef> cfCredential = adoptCF(createCF(credential));
> -        CFURLConnectionUseCredential(d->m_connection.get(), cfCredential.get(), challenge.cfURLAuthChallengeRef());
> -#endif

Here you kept the Cocoa version. I think that’s backwards.

> Source/WebKit/win/WebView.cpp:5264
> +#if PLATFORM(WIN) || USE(CFURLCONNECTION)

This should just be PLATFORM(WIN), no need for the USE(CFURLCONNECTION), right?
Comment 11 Alex Christensen 2016-10-11 08:23:08 PDT
Comment on attachment 291232 [details]
Patch

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

>> Source/WebCore/platform/network/ProtectionSpaceBase.cpp:-33
>> -#endif
> 
> Why isn’t this needed on Windows?

It compiles without it.  It must be left over from code that used to be in this file.
Comment 12 Alex Christensen 2016-10-11 08:24:02 PDT
Created attachment 291259 [details]
Patch
Comment 13 WebKit Commit Bot 2016-10-11 08:25:55 PDT
Attachment 291259 [details] did not pass style-queue:


ERROR: Source/WebCore/WebCorePrefix.h:0:  Use #pragma once header guard.  [build/header_guard] [5]
ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:0:  Use #pragma once header guard.  [build/header_guard] [5]
Total errors found: 2 in 87 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 Alex Christensen 2016-10-11 09:40:46 PDT
Created attachment 291270 [details]
Patch
Comment 15 WebKit Commit Bot 2016-10-11 09:43:11 PDT
Attachment 291270 [details] did not pass style-queue:


ERROR: Source/WebCore/WebCorePrefix.h:0:  Use #pragma once header guard.  [build/header_guard] [5]
ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:0:  Use #pragma once header guard.  [build/header_guard] [5]
Total errors found: 2 in 87 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 16 Daniel Bates 2016-10-11 10:14:48 PDT
Comment on attachment 291270 [details]
Patch

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

Yay!

> Source/WebCore/platform/network/cf/AuthenticationCF.cpp:89
> +    return CFURLAuthChallengeCreate(0, protectionSpace.get(), credential.get(), coreChallenge.previousFailureCount(), coreChallenge.failureResponse().cfURLResponse(), coreChallenge.error());

Nit: 0 => nullptr

> Source/WebCore/platform/network/cf/NetworkStorageSessionCFNet.cpp:41
> +// FIXME: This file is mostly Cocoa code, not CFNet code.

CFNet => CFNetwork

We should look to extract out the CFNetwork code an rename/move this file to reflect that it is used by Cocoa-based platforms. We can do that in a separate bug/patch.
Comment 17 Alex Christensen 2016-10-11 11:16:24 PDT
Also removing ResourceRequestMac.mm
Comment 18 Alex Christensen 2016-10-11 11:23:22 PDT
http://trac.webkit.org/changeset/207151