Bug 215940

Summary: Web Inspector: Graphics: remove unnecessary main page check when iterating existing animations
Product: WebKit Reporter: Devin Rousso <hi>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED WONTFIX    
Severity: Normal CC: bburg, hi, inspector-bugzilla-changes, joepeck, pangle, timothy
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 208493    
Bug Blocks:    
Attachments:
Description Flags
Patch bburg: review-, bburg: commit-queue-

Devin Rousso
Reported 2020-08-28 11:32:38 PDT
.
Attachments
Patch (4.96 KB, patch)
2020-08-28 11:38 PDT, Devin Rousso
bburg: review-
bburg: commit-queue-
Devin Rousso
Comment 1 2020-08-28 11:38:46 PDT
Blaze Burg
Comment 2 2020-08-28 12:12:02 PDT
Comment on attachment 407491 [details] Patch LGTM, but it doesn't build. Is there a behavior change? Does this mean iframe animations will show in the main page? or that WebAnimation::instances() only returns such animations?
Devin Rousso
Comment 3 2020-08-28 13:19:43 PDT
(In reply to Brian Burg from comment #2) > Comment on attachment 407491 [details] > Patch > > LGTM, but it doesn't build. grumble mumble `-Wunused-private-field` grumble grumble > Is there a behavior change? Does this mean iframe animations will show in the main page? or that WebAnimation::instances() only returns such animations? AFAIU this is not a behavior change as subframes share the same `Page` as the main frame. It's already possible to see `WebAnimation` inside `<iframe>`. I'll adjust the ChangeLog comment to further clarify that this code indeed basically did nothing. In fact, it's possible to see `WebSocket` and `<canvas>` too, so I think we can remove those checks as well :)
Devin Rousso
Comment 4 2020-08-28 13:28:42 PDT
I totally forgot that there can be more than one `Page` per WebProcess, so we actually still do need this code.
Note You need to log in before you can comment on or make changes to this bug.