Bug 200886

Summary: [SOUP] NetworkProcessSoup does not initialize CacheOptions correctly
Product: WebKit Reporter: enometh
Component: WebKitGTKAssignee: Claudio Saavedra <csaavedra>
Status: RESOLVED FIXED    
Severity: Normal CC: aperez, bugs-noreply, cgarcia, commit-queue, csaavedra, pnormand, zan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
sketch
none
Patch
none
Patch none

enometh
Reported 2019-08-19 06:44:28 PDT
Created attachment 376683 [details] sketch git commit 464bf0 http://svn.webkit.org/repository/webkit/trunk@247567 overlooked propagating the cacheOptions which are set in WebKit::NetworkProcess::platformInitializeNetworkProcess The attached patch outlines a fix
Attachments
sketch (1.37 KB, patch)
2019-08-19 06:44 PDT, enometh 		no flags
DetailsFormatted DiffDiff
Patch (2.08 KB, patch)
2019-08-22 01:39 PDT, Claudio Saavedra 		no flags
DetailsFormatted DiffDiff
Patch (2.08 KB, patch)
2019-08-22 01:40 PDT, Claudio Saavedra 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
enometh
Comment 1 2019-08-19 06:59:03 PDT
(The attached patch unnecessarily marked the slot on NetworkProcess as protected . It shouldn't I hope this does not confuse the main issue.)
Claudio Saavedra
Comment 2 2019-08-22 00:59:45 PDT
Comment on attachment 376683 [details] sketch View in context: https://bugs.webkit.org/attachment.cgi?id=376683&action=review > Source/WebKit/NetworkProcess/soup/NetworkProcessSoup.cpp:121 > + cacheOptions.add(NetworkCache::CacheOption::RegisterNotify); I'm confused by the original code here. I don't see this cacheOptions local variable used anywhere, only set here but never read. Is this correct at all or am I missing something?
Claudio Saavedra
Comment 3 2019-08-22 01:01:30 PDT
I think the code must have been wrong already before achristensen's patch. Here we should be setting m_cacheOptions, not a local variable.
Claudio Saavedra
Comment 4 2019-08-22 01:39:02 PDT
Created attachment 376996 [details] Patch
Claudio Saavedra
Comment 5 2019-08-22 01:40:37 PDT
Created attachment 376997 [details] Patch
Adrian Perez
Comment 6 2019-08-22 01:46:54 PDT
Comment on attachment 376997 [details] Patch r+, patch LGTM (reviewing informally)
WebKit Commit Bot
Comment 7 2019-08-22 02:40:32 PDT
Comment on attachment 376997 [details] Patch Clearing flags on attachment: 376997 Committed r248999: <https://trac.webkit.org/changeset/248999>
WebKit Commit Bot
Comment 8 2019-08-22 02:40:33 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.