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

Peng Liu
Reported 2020-04-09 11:05:53 PDT
Add null pointer checking in WebFullScreenManager
Attachments
Patch (4.84 KB, patch)
2020-04-09 11:11 PDT, Peng Liu 		dbates: review+
DetailsFormatted DiffDiff
Patch for landing (4.47 KB, patch)
2020-04-09 22:20 PDT, Peng Liu 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Peng Liu
Comment 1 2020-04-09 11:07:28 PDT
<rdar://problem/54461890>
Peng Liu
Comment 2 2020-04-09 11:11:38 PDT
Created attachment 395977 [details] Patch
Daniel Bates
Comment 3 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.
Peng Liu
Comment 4 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.
Peng Liu
Comment 5 2020-04-09 22:20:29 PDT
Created attachment 396047 [details] Patch for landing
EWS
Comment 6 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].
Note You need to log in before you can comment on or make changes to this bug.