Bug 210291

Summary:

Add null pointer checkings in WebFullScreenManager

Product:

WebKit

Reporter:

Peng Liu <peng.liu6>

Component:

Media

Assignee:

Peng Liu <peng.liu6>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eric.carlson, jer.noble, webkit-bug-importer

Priority:

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].