[Webkit-unassigned] [Bug 208612] Resource Load Statistics: Keep a count of resource loads per day

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 4 17:16:32 PST 2020


--- Comment #3 from katherine_cheney at apple.com ---
Comment on attachment 392506
  --> https://bugs.webkit.org/attachment.cgi?id=392506

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

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:86
> +constexpr auto insertResourceLoadCountRowQuery = "INSERT OR IGNORE INTO ResourceLoadCount VALUES(?, 0)"_s;

INSERT OR IGNORE calls might require creating a unique index. Maybe not though because ResourceLoadCount has a primary key. Did you test that repeat attempts don't create a new row?

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:356
> +            if (strncmp(table, "ResourceLoadCount", strlen(table))) {

You can just use == here.

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:359
> +            } else if (!createResourceLoadCountTable()) {

Hmm, this seems strange. createResourceLoadCountTable() will be called for every other table iteration besides ResourceLoadCount. It should probably only be called once.

I think this could be re-written in a way to  make it easier to add new tables in the future. Maybe maintain a list of missing tables, and add them at the end?

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:602
> +

You can use resetStatement() here if you want. One less line of code.

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:620
> +    ASSERT_UNUSED(resetResult, resetResult == SQLITE_OK);


> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:645
> +


> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:1346
> +        buildList(WTF::IteratorRange<Vector<unsigned>::iterator>(julianDaysInRange.begin(), julianDaysInRange.end())), ") ORDER BY day"));

I think theres a static buildList function already in the class you can use.

> Source/WebKit/Shared/WebCoreArgumentCoders.cpp:2862

Maybe I'm out of the loop, but what does "Transient stats" mean here?

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200305/2d093e65/attachment.htm>

More information about the webkit-unassigned mailing list