Bug 210291 – Add null pointer checkings in WebFullScreenManager

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Peng Liu]

Add null pointer checking in WebFullScreenManager

[Peng Liu]

<rdar://problem/54461890>

[Peng Liu]

Created attachment 395977 [details]
Patch

[Daniel Bates]

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.

[Peng Liu]

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.

[Peng Liu]

Created attachment 396047 [details]
Patch for landing

[EWS]

Committed r259874: <https://trac.webkit.org/changeset/259874>

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