RESOLVED FIXED 199327
Null check provisionalItem in FrameLoader::continueLoadAfterNavigationPolicy
https://bugs.webkit.org/show_bug.cgi?id=199327
Summary Null check provisionalItem in FrameLoader::continueLoadAfterNavigationPolicy
Alex Christensen
Reported 2019-06-28 11:09:10 PDT
Null check provisionalItem in FrameLoader::continueLoadAfterNavigationPolicy
Attachments
Patch (1.85 KB, patch)
2019-06-28 11:10 PDT, Alex Christensen
no flags
Alex Christensen
Comment 1 2019-06-28 11:10:06 PDT
Alex Christensen
Comment 2 2019-06-28 11:10:08 PDT
Daniel Bates
Comment 3 2019-06-28 23:16:49 PDT
Comment on attachment 373135 [details] Patch So, I’ve been out of this code for a while. But I spent like ***20**** seconds just grepping this file for provisionalItem and I can say that this patch makes absolutely no sense to me. Hint: It contradicts a bunch of RELEASE_ASSERTS. And with 0 details about why this fix is necessary except an appeal to “Let’s not crash” I have no means of helping you further (I don’t have a radar client handy at the moment).
Maciej Stachowiak
Comment 4 2019-07-01 02:23:21 PDT
This is an attempt to fix a top crash based on telemetry. It doesn't contradict a RELEASE_ASSERT in the control flow path that led to the crash, because otherwise these crashes would have crashed earlier. The crash is known to be a null Derek on this line. So this is probably sensible as a speculative fix, even though there's probably something deeper wrong here.
Ryosuke Niwa
Comment 5 2019-07-01 13:17:43 PDT
(In reply to Daniel Bates from comment #3) > Comment on attachment 373135 [details] > Patch > > So, I’ve been out of this code for a while. But I spent like ***20**** > seconds just grepping this file for provisionalItem and I can say that this > patch makes absolutely no sense to me. Hint: It contradicts a bunch of > RELEASE_ASSERTS. And with 0 details about why this fix is necessary except > an appeal to “Let’s not crash” I have no means of helping you further (I > don’t have a radar client handy at the moment). That is true but we've been trying to fix this particular crash for the last 2-3 releases and at some point, we need to fix the damn crash for the sake of our users instead of keep investigating what the root cause is.
WebKit Commit Bot
Comment 6 2019-07-01 14:11:47 PDT
Comment on attachment 373135 [details] Patch Clearing flags on attachment: 373135 Committed r247021: <https://trac.webkit.org/changeset/247021>
WebKit Commit Bot
Comment 7 2019-07-01 14:11:48 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.