Bug 210291

Summary: Add null pointer checkings in WebFullScreenManager
Product: WebKit Reporter: Peng Liu <peng.liu6>
Component: MediaAssignee: Peng Liu <peng.liu6>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, jer.noble, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
dbates: review+
Patch for landing none

Description Peng Liu 2020-04-09 11:05:53 PDT
Add null pointer checking in WebFullScreenManager
Comment 1 Peng Liu 2020-04-09 11:07:28 PDT
<rdar://problem/54461890>
Comment 2 Peng Liu 2020-04-09 11:11:38 PDT
Created attachment 395977 [details]
Patch
Comment 3 Daniel Bates 2020-04-09 20:39:34 PDT
Comment on attachment 395977 [details]
Patch

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

Patch looks good. A better patch would explain why this is being done in the change log description.

> Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp:54
> +    if (!element)

Ok as is. No change needed. Is the purpose of this change to workaround an unreproducible bug? Is so then the the optimal solution would include a FIXME comment explaining this/referencing the bug for the root cause fix. If not, then The optimal solution is to remove the assert.

Same comment for all changes below.
Comment 4 Peng Liu 2020-04-09 20:56:16 PDT
Comment on attachment 395977 [details]
Patch

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

>> Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp:54
>> +    if (!element)
> 
> Ok as is. No change needed. Is the purpose of this change to workaround an unreproducible bug? Is so then the the optimal solution would include a FIXME comment explaining this/referencing the bug for the root cause fix. If not, then The optimal solution is to remove the assert.
> 
> Same comment for all changes below.

Thanks for the review.
Right, this patch is to fix a hard to reproduce crash.
Comment 5 Peng Liu 2020-04-09 22:20:29 PDT
Created attachment 396047 [details]
Patch for landing
Comment 6 EWS 2020-04-10 08:54:56 PDT
Committed r259874: <https://trac.webkit.org/changeset/259874>

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