Bug 192592

Summary: Resource Load Statistics: Use common implementation within NetworkStorageSession
Product: WebKit Reporter: Don Olmstead <don.olmstead>
Component: WebKit Misc.Assignee: Don Olmstead <don.olmstead>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, bugs-noreply, cdumez, commit-queue, mcatanzaro, pgriffis, webkit-bug-importer, wilander
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Don Olmstead
Reported 2018-12-11 09:12:22 PST
Currently there is a common implementation of ENABLE(RESOURCE_LOAD_STATISTICS) within NetworkStorageSessionCFNet.cpp that should go into NetworkStorageSession.
Attachments
Patch (20.92 KB, patch)
2018-12-11 09:20 PST, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Patch (20.88 KB, patch)
2018-12-11 09:41 PST, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Don Olmstead
Comment 1 2018-12-11 09:20:04 PST
Created attachment 357054 [details] Patch
Don Olmstead
Comment 2 2018-12-11 09:41:18 PST
Created attachment 357060 [details] Patch
Michael Catanzaro
Comment 3 2018-12-11 09:47:49 PST
Yay!
Alex Christensen
Comment 4 2018-12-11 11:47:34 PST
Comment on attachment 357060 [details] Patch r=me You should be aware that the high-level organization of resource load statistics is not where we want it to be. I think right now it uses some strange combination of process pool and globals, and it needs some love and polish.
Michael Catanzaro
Comment 5 2018-12-11 12:15:04 PST
Comment on attachment 357060 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=357060&action=review > Source/WebCore/platform/network/NetworkStorageSession.cpp:106 > +bool NetworkStorageSession::shouldBlockCookies(const ResourceRequest& request, std::optional<uint64_t> frameID, std::optional<uint64_t> pageID) const Preexisting problem, but I presume passing std::optional by value is not best-practice? Should be const ref?
Chris Dumez
Comment 6 2018-12-11 12:17:44 PST
(In reply to Michael Catanzaro from comment #5) > Comment on attachment 357060 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=357060&action=review > > > Source/WebCore/platform/network/NetworkStorageSession.cpp:106 > > +bool NetworkStorageSession::shouldBlockCookies(const ResourceRequest& request, std::optional<uint64_t> frameID, std::optional<uint64_t> pageID) const > > Preexisting problem, but I presume passing std::optional by value is not > best-practice? Should be const ref? I think we're OK with it for small types.
WebKit Commit Bot
Comment 7 2018-12-11 12:25:30 PST
Comment on attachment 357060 [details] Patch Clearing flags on attachment: 357060 Committed r239081: <https://trac.webkit.org/changeset/239081>
WebKit Commit Bot
Comment 8 2018-12-11 12:25:32 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9 2018-12-11 12:26:36 PST
<rdar://problem/46637456>
Note You need to log in before you can comment on or make changes to this bug.