Bug 212916 – ResourceLoadStatisticsDatabaseStore::domainIDFromString failed, error message: bad parameter or other API misuse

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

[Kate Cheney]

The ResourceLoadStatisticsDatabaseStore is reporting "bad parameter or other API misuse" errors.

[Kate Cheney]

<rdar://problem/64127238>

[Kate Cheney]

Created attachment 401364 [details]
Patch

[Sihui Liu]

Comment on attachment 401364 [details]
Patch

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

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:2226
> +    // Reset this statement because it will be used again in the same scope
> +    // when calling insertObservedDomain.
> +    scopedStatement->reset();

Can we put the scopedStatement in scope stead? like:
{
  auto scopedStatement = this->scopedStatement(m_domainIDFromStringStatement, domainIDFromStringQuery, "ensureResourceStatisticsForRegistrableDomain"_s);
    if (!scopedStatement
        || scopedStatement->bindText(1, domain.string()) != SQLITE_OK) {
        RELEASE_LOG_ERROR_IF_ALLOWED(m_sessionID, "%p - ResourceLoadStatisticsDatabaseStore::ensureResourceStatisticsForRegistrableDomain failed, error message: %{private}s", this, m_database.lastErrorMsg());
        ASSERT_NOT_REACHED();
        return { AddedRecord::No, 0 };
    }
    
    if (scopedStatement->step() == SQLITE_ROW) {
        unsigned domainID = scopedStatement->getColumnInt(0);
        return { AddedRecord::No, domainID };
    }
}

[Kate Cheney]

Created attachment 401376 [details]
Patch

[Kate Cheney]

(In reply to Sihui Liu from comment #3)
> Comment on attachment 401364 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=401364&action=review
> 
> > Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:2226
> > +    // Reset this statement because it will be used again in the same scope
> > +    // when calling insertObservedDomain.
> > +    scopedStatement->reset();
> 
> Can we put the scopedStatement in scope stead? like:
> {
>   auto scopedStatement =
> this->scopedStatement(m_domainIDFromStringStatement,
> domainIDFromStringQuery, "ensureResourceStatisticsForRegistrableDomain"_s);
>     if (!scopedStatement
>         || scopedStatement->bindText(1, domain.string()) != SQLITE_OK) {
>         RELEASE_LOG_ERROR_IF_ALLOWED(m_sessionID, "%p -
> ResourceLoadStatisticsDatabaseStore::
> ensureResourceStatisticsForRegistrableDomain failed, error message:
> %{private}s", this, m_database.lastErrorMsg());
>         ASSERT_NOT_REACHED();
>         return { AddedRecord::No, 0 };
>     }
>     
>     if (scopedStatement->step() == SQLITE_ROW) {
>         unsigned domainID = scopedStatement->getColumnInt(0);
>         return { AddedRecord::No, domainID };
>     }
> }

Yes, probably a better idea, I did this in the latest patch.

[Sihui Liu]

Comment on attachment 401376 [details]
Patch

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

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:2211
> +{

Nit: indent the block

[Kate Cheney]

Created attachment 401392 [details]
Patch for landing

[EWS]

Committed r262752: <https://trac.webkit.org/changeset/262752>

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

[Radar WebKit Bug Importer]

<rdar://problem/64140844>