Bug 215940 - Web Inspector: Graphics: remove unnecessary main page check when iterating existing animations
Summary: Web Inspector: Graphics: remove unnecessary main page check when iterating ex...
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords:
Depends on: 208493
Blocks:
  Show dependency tree / graph
 
Reported: 2020-08-28 11:32 PDT by Devin Rousso
Modified: 2020-08-28 13:28 PDT (History)
6 users (show)

See Also:

Attachments
Patch (4.96 KB, patch)
2020-08-28 11:38 PDT, Devin Rousso 		bburg: review-
bburg: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2020-08-28 11:32:38 PDT 
.
Comment 1 Devin Rousso 2020-08-28 11:38:46 PDT 
Created attachment 407491 [details]
Patch
Comment 2 BJ Burg 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?
Comment 3 Devin Rousso 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 :)
Comment 4 Devin Rousso 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.