Bug 219860

Summary: CrashTracer: com.apple.WebKit.Networking at WebKit: WTF::Detail::CallableWrapper<WebKit::PrivateClickMeasurementManager::firePendingAttributionRequests()
Product: WebKit Reporter: Kate Cheney <katherine_cheney>
Component: WebKit Misc.Assignee: Kate Cheney <katherine_cheney>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, ddkilzer, webkit-bug-importer, wilander
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Kate Cheney
Reported 2020-12-14 10:22:29 PST
We are hitting this crash after PCM code was migrated to use SQLite.
Attachments
Patch (3.96 KB, patch)
2020-12-14 10:55 PST, Kate Cheney
no flags
Patch (6.98 KB, patch)
2020-12-14 12:10 PST, Kate Cheney
no flags
Patch (7.43 KB, patch)
2020-12-15 08:54 PST, Kate Cheney
no flags
Kate Cheney
Comment 1 2020-12-14 10:55:52 PST
Kate Cheney
Comment 2 2020-12-14 10:56:28 PST
Alex Christensen
Comment 3 2020-12-14 11:53:23 PST
Comment on attachment 416172 [details] Patch From the description, I think this should just add this: if (!m_networkSession) return;
Kate Cheney
Comment 4 2020-12-14 11:58:27 PST
(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
Comment 5 2020-12-14 11:59:19 PST
if (auto* resourceLoadStatistics = m_networkSession ? m_networkSession->resourceLoadStatistics() : nullptr) checks m_networkSession, but what you have doesn't.
Kate Cheney
Comment 6 2020-12-14 12:10:19 PST
EWS
Comment 7 2020-12-14 12:59:14 PST
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
Comment 8 2020-12-14 13:01:44 PST
John Wilander
Comment 9 2020-12-14 17:34:23 PST
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 10 2020-12-14 18:25:13 PST
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
Comment 11 2020-12-15 08:54:02 PST
Reopening to attach new patch.
Kate Cheney
Comment 12 2020-12-15 08:54:03 PST
EWS
Comment 13 2020-12-15 10:28:21 PST
Committed r270852: <https://trac.webkit.org/changeset/270852> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416253 [details].
Note You need to log in before you can comment on or make changes to this bug.