Bug 177424 - Remove ENABLE_NETWORK_CACHE
Summary: Remove ENABLE_NETWORK_CACHE
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Don Olmstead
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-09-25 00:16 PDT by Yoshiaki Jitsukawa
Modified: 2017-10-11 08:54 PDT (History)
9 users (show)

See Also:


Attachments
Patch (46.26 KB, patch)
2017-10-10 14:22 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (46.52 KB, patch)
2017-10-10 17:36 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yoshiaki Jitsukawa 2017-09-25 00:16:36 PDT
https://trac.webkit.org/changeset/221942/webkit introduced NetworkCache::Key and other things in NetworkCache namespace
into WebKit/NetworkProcess/cache/CacheStorageEngineCache, but NetworkCache::* is not defined unless ENABLE(NETWORK_CACHE) is true
and this causes WebKit to fail to build.

The definition in WebKit/config.h is 
#ifndef ENABLE_NETWORK_CACHE
#if PLATFORM(COCOA) || USE(SOUP)
#define ENABLE_NETWORK_CACHE 1
#else
#define ENABLE_NETWORK_CACHE 0
#endif
#endif

and it seems like all webkit(2) port enable it today.

Should we support the case ENABLE_NETWORK_CACHE==0 or should all webkit(2) port (including upcoming windows one) enable network cache?
Comment 1 Antti Koivisto 2017-09-25 09:52:29 PDT
We should just remove ENABLE_NETWORK_CACHE define and always compile in the code. Cache can still be disabled dynamically.
Comment 2 Yoshiaki Jitsukawa 2017-09-25 14:54:53 PDT
(In reply to Antti Koivisto from comment #1)
> We should just remove ENABLE_NETWORK_CACHE define and always compile in the
> code. Cache can still be disabled dynamically.

I understand. Thank you for the clarification :)
Comment 3 Don Olmstead 2017-10-10 14:22:00 PDT
Created attachment 323349 [details]
Patch
Comment 4 Antti Koivisto 2017-10-10 15:02:13 PDT
Comment on attachment 323349 [details]
Patch

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

> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:320
>      bool shouldSendDidReceiveResponse = true;

This line can be removed

> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:334
>      shouldSendDidReceiveResponse = !m_cacheEntryForValidation;

... and this turned into

bool shouldSendDidReceiveResponse = !m_cacheEntryForValidation

> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:-362
>      // For main resources, the web process is responsible for sending back a NetworkResourceLoader::ContinueDidReceiveResponse message.
>      bool shouldContinueDidReceiveResponse = !shouldWaitContinueDidReceiveResponse;
> -#if ENABLE(NETWORK_CACHE)
>      shouldContinueDidReceiveResponse = shouldContinueDidReceiveResponse || m_cacheEntryForValidation;
> -#endif

This can be reduced to

bool shouldContinueDidReceiveResponse = !shouldWaitContinueDidReceiveResponse || m_cacheEntryForValidation;
Comment 5 Don Olmstead 2017-10-10 17:36:32 PDT
Created attachment 323365 [details]
Patch

Fixing review comments
Comment 6 Antti Koivisto 2017-10-11 00:01:59 PDT
Comment on attachment 323365 [details]
Patch

r=me
Comment 7 WebKit Commit Bot 2017-10-11 08:53:25 PDT
Comment on attachment 323365 [details]
Patch

Clearing flags on attachment: 323365

Committed r223179: <https://trac.webkit.org/changeset/223179>
Comment 8 WebKit Commit Bot 2017-10-11 08:53:27 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2017-10-11 08:54:09 PDT
<rdar://problem/34934064>