Bug 230839 – PCM: different bundleID entries will override each other

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Kate Cheney]

We should make bundleID a part of the unique constraint of PCM tables.

[Radar WebKit Bug Importer]

<rdar://problem/83575086>

[Kate Cheney]

Created attachment 439357 [details]
Patch

[Kate Cheney]

Created attachment 439387 [details]
Patch

[Alex Christensen]

Comment on attachment 439387 [details]
Patch

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

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:276
> +const Vector<String>& ResourceLoadStatisticsDatabaseStore::sortedTables()

This could be a std::array, which would not require any dynamic allocation.

> Source/WebKit/NetworkProcess/DatabaseUtilities.cpp:199
> +        RELEASE_LOG_ERROR(Network, "%p - ResourceLoadStatisticsDatabaseStore::currentTableAndIndexQueries Unable to prepare statement to fetch schema for the table, error message: %" PRIVATE_LOG_STRING, this, m_database.lastErrorMsg());

DatabaseUtilities::
Also, I've been using the PrivateClickMeasurement logging channel in this file even though it's shared with RLS.  Network isn't a great channel for this.
Ditto several times below.

> Source/WebKit/NetworkProcess/DatabaseUtilities.h:43
> +typedef std::pair<String, std::optional<String>> TableAndIndexPair;

using TableAndIndexPair = std::pair<String, std::optional<String>>;

[Kate Cheney]

Created attachment 439404 [details]
Patch for landing

[Kate Cheney]

(In reply to Alex Christensen from comment #4)
> Comment on attachment 439387 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=439387&action=review
> 
> > Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:276
> > +const Vector<String>& ResourceLoadStatisticsDatabaseStore::sortedTables()
> 
> This could be a std::array, which would not require any dynamic allocation.
> 

Did not do this because the function is virtual.

> > Source/WebKit/NetworkProcess/DatabaseUtilities.cpp:199
> > +        RELEASE_LOG_ERROR(Network, "%p - ResourceLoadStatisticsDatabaseStore::currentTableAndIndexQueries Unable to prepare statement to fetch schema for the table, error message: %" PRIVATE_LOG_STRING, this, m_database.lastErrorMsg());
> 
> DatabaseUtilities::
> Also, I've been using the PrivateClickMeasurement logging channel in this
> file even though it's shared with RLS.  Network isn't a great channel for
> this.
> Ditto several times below.
> 

Fixed.

> > Source/WebKit/NetworkProcess/DatabaseUtilities.h:43
> > +typedef std::pair<String, std::optional<String>> TableAndIndexPair;
> 
> using TableAndIndexPair = std::pair<String, std::optional<String>>;

Fixed.

[Kate Cheney]

Comment on attachment 439404 [details]
Patch for landing

Cancelling commit queue to investigate failure.

[EWS]

Found 1 new test failure: http/tests/privateClickMeasurement/expired-attribution-report-gets-sent-on-session-start.html

[Kate Cheney]

Created attachment 439431 [details]
Patch

[Kate Cheney]

Created attachment 439511 [details]
Patch

[Kate Cheney]

Changed REFERENCES ObservedDomains(domainID) ON DELETE CASCADE -> REFERENCES PCMObservedDomains(domainID) ON DELETE CASCADE in the test case. This was causing a false positive pass. Now this test will fail without the accompanying changes.

[EWS]

Committed r283194 (242241@main): <https://commits.webkit.org/242241@main>

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