CrashTracer: com.apple.WebKit.Networking at WebKit: WTF::Detail::CallableWrapper<WebKit::PrivateClickMeasurementManager::firePendingAttributionRequests()

[Kate Cheney]

We are hitting this crash after PCM code was migrated to use SQLite.

[Kate Cheney]

Created attachment 416172 [details]
Patch

[Kate Cheney]

<rdar://problem/72254207>

[Alex Christensen]

Comment on attachment 416172 [details]
Patch

From the description, I think this should just add this:
if (!m_networkSession)
    return;

[Kate Cheney]

(In reply to Alex Christensen from comment #3)
> Comment on attachment 416172 [details]
> Patch
> 
> From the description, I think this should just add this:
> if (!m_networkSession)
>     return;

I thought if (auto* resourceLoadStatistics = m_networkSession->resourceLoadStatistics()) would check m_networkSession but maybe I am wrong? If so I should check m_networkSession in other PrivateClickMeasurementManager functions to be consistent.

[Alex Christensen]

if (auto* resourceLoadStatistics = m_networkSession ? m_networkSession->resourceLoadStatistics() : nullptr) checks m_networkSession, but what you have doesn't.

[Kate Cheney]

Created attachment 416180 [details]
Patch

[EWS]

Committed r270802: <https://trac.webkit.org/changeset/270802>

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

[Radar WebKit Bug Importer]

<rdar://problem/72310410>

[John Wilander]

Comment on attachment 416180 [details]
Patch

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

An easier way to fix this is to move the null check into PrivateClickMeasurementManager::featureEnabled().

> Source/WebKit/NetworkProcess/PrivateClickMeasurementManager.cpp:302
> +        return;

We need to also call the completion handler here.

> Source/WebKit/NetworkProcess/PrivateClickMeasurementManager.cpp:342
> +        return;

We need to also call the completion handler here.

[Alex Christensen]

Comment on attachment 416180 [details]
Patch

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

>> Source/WebKit/NetworkProcess/PrivateClickMeasurementManager.cpp:302
>> +        return;
> 
> We need to also call the completion handler here.

yes we certainly do.  Also in the case before if resourceLoadStatistics is null.

[Kate Cheney]

Reopening to attach new patch.

[Kate Cheney]

Created attachment 416253 [details]
Patch

[EWS]

Committed r270852: <https://trac.webkit.org/changeset/270852>

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