Bug 225063 – ResourceLoadStatisticsDatabaseStore::aggregatedThirdPartyData() hangs for a long time

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Kate Cheney]

ResourceLoadStatisticsDatabaseStore::aggregatedThirdPartyData() hangs for a long time

[Kate Cheney]

<rdar://problem/76248557>

[Kate Cheney]

Created attachment 427076 [details]
Patch

[Brent Fulgham]

Comment on attachment 427076 [details]
Patch

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

Do we need to check in all three of these files? I thought that the *.db-shm and *.db-wal were dynamic byproducts of loading the sqlite database?

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:305
> +static const MemoryCompactLookupOnlyRobinHoodHashMap<String, std::pair<String, Optional<String>>>& createTableAndIndexQueries()

Some of this would be easier to read with a type declaration:

typedef std::pair<String, Optional<String>> TableAndIndexPair

.. or something similar.

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:437
> +std::pair<String, Optional<String>> ResourceLoadStatisticsDatabaseStore::tableSchemaAndIndex(const String& tableName)

I wonder if this should be called "tableAndIndexSchema"?

[Brent Fulgham]

Comment on attachment 427076 [details]
Patch

r=me, but please confirm we need to check in all three files. I also think that creating a typedef for the table and index schema would make the code easier to read and understand.

[Kate Cheney]

Comment on attachment 427076 [details]
Patch

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

Hmm, good point about the 3 files. Let me run the test without them to be sure, but seems likely they could be removed. Thanks for the comments!

>> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:305
>> +static const MemoryCompactLookupOnlyRobinHoodHashMap<String, std::pair<String, Optional<String>>>& createTableAndIndexQueries()
> 
> Some of this would be easier to read with a type declaration:
> 
> typedef std::pair<String, Optional<String>> TableAndIndexPair
> 
> .. or something similar.

Good call, will change.

>> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:437
>> +std::pair<String, Optional<String>> ResourceLoadStatisticsDatabaseStore::tableSchemaAndIndex(const String& tableName)
> 
> I wonder if this should be called "tableAndIndexSchema"?

Yes, the name is a bit awkward. Maybe even "tableAndIndexQueries" would be better, because that's really what the data is. We are using the queries to imply the schema.

[Kate Cheney]

Created attachment 427098 [details]
Patch

[EWS]

Committed r276620 (237049@main): <https://commits.webkit.org/237049@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427098 [details].

[Radar WebKit Bug Importer]

<rdar://problem/77182578>