Further WebResourceLoadStatisticsStore / ResourceLoadStatisticsStore clean up

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

Further WebResourceLoadStatisticsStore / ResourceLoadStatisticsStore clean up.

[Chris Dumez]

Created attachment 314953 [details]
Patch

[Build Bot]

Attachment 314953 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.cpp:257:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.h:74:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 314954 [details]
Patch

[Build Bot]

Attachment 314954 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.cpp:257:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.h:74:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 314955 [details]
Patch

[Build Bot]

Attachment 314955 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.cpp:257:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.h:74:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 314983 [details]
Patch

[Build Bot]

Attachment 314983 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.cpp:257:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.h:74:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brent Fulgham]

Comment on attachment 314983 [details]
Patch

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

r=me, with some minor comments.

> Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.cpp:173
> +            return WTFMove(statistic);

Very nice!

> Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.cpp:-322
> -}

I wish the code review tool represented moved code better. It looks like this was lifted to the WK2 layer.

> Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp:-156
> -    // Switch to the main thread to get the default website data store

Is this not true anymore? I can't remember if your other changes ensured that we always were interacting with the right website data store. If you did, then is it really necessary to move to the main runloop?

If your changes did not tie us to the main data store, I think this comment is still useful.

> Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp:291
> +    // FIXME: Decide what to call this file.

I don't think we're going to change the name at this point. Let's just remove this comment.

[Chris Dumez]

Comment on attachment 314983 [details]
Patch

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

>> Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.cpp:-322
>> -}
> 
> I wish the code review tool represented moved code better. It looks like this was lifted to the WK2 layer.

It was already in WebKit2, I merely moved it to our Telemetry file / namespace.

>> Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp:-156
>> -    // Switch to the main thread to get the default website data store
> 
> Is this not true anymore? I can't remember if your other changes ensured that we always were interacting with the right website data store. If you did, then is it really necessary to move to the main runloop?
> 
> If your changes did not tie us to the main data store, I think this comment is still useful.

This comment is not a "why" comment IMHO. It basically states what the code does which is already obvious by reading the code. AFAIK, we do not have such comments in WebKit, which is why I dropped it.

>> Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp:291
>> +    // FIXME: Decide what to call this file.
> 
> I don't think we're going to change the name at this point. Let's just remove this comment.

Ok.

[Chris Dumez]

Created attachment 314987 [details]
Patch

[Build Bot]

Attachment 314987 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.cpp:257:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.h:74:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 314987 [details]
Patch

Clearing flags on attachment: 314987

Committed r219297: <http://trac.webkit.org/changeset/219297>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.