Bug 189130

Summary: Avoid code duplication in ResourceLoadStatisticsMemoryStore::processStatisticsAndDataRecords()
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, webkit-bug-importer, wilander, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Chris Dumez 2018-08-29 16:49:21 PDT 
Avoid code duplication in ResourceLoadStatisticsMemoryStore::processStatisticsAndDataRecords().
Comment 1 Chris Dumez 2018-08-29 16:50:37 PDT 
Created attachment 348449 [details]
Patch
Comment 2 youenn fablet 2018-08-29 17:10:04 PDT 
Comment on attachment 348449 [details]
Patch

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

> Source/WebKit/UIProcess/ResourceLoadStatisticsMemoryStore.cpp:358
> +            return;

I guess there is a small behavior change if m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned value is updated between the time removeDataRecords is called and the lambda is executed.
Comment 3 WebKit Commit Bot 2018-08-29 20:40:06 PDT 
Comment on attachment 348449 [details]
Patch

Clearing flags on attachment: 348449

Committed r235495: <https://trac.webkit.org/changeset/235495>
Comment 4 WebKit Commit Bot 2018-08-29 20:40:08 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 5 Radar WebKit Bug Importer 2018-08-29 20:41:19 PDT 
<rdar://problem/43871000>
Comment 6 John Wilander 2018-08-30 08:48:53 PDT 
Comment on attachment 348449 [details]
Patch

If I recall correctly, this didn't use to be a pure duplication. There was something about the order in which things were done. But now it is totally a duplication and the patch looks good to me.

(The code has been moved and changed too much for me to easily find out what the difference used to be.)
Comment 7 Chris Dumez 2018-08-30 08:52:07 PDT 
(In reply to John Wilander from comment #6)
> Comment on attachment 348449 [details]
> Patch
> 
> If I recall correctly, this didn't use to be a pure duplication. There was
> something about the order in which things were done. But now it is totally a
> duplication and the patch looks good to me.
> 
> (The code has been moved and changed too much for me to easily find out what
> the difference used to be.)

Ok, that makes sense. This code has been refactored a lot. Thanks for checking.