Bug 179131 - [Performance] Painting <video> to canvas spends a lot of time in URL getting and parsing
Summary: [Performance] Painting <video> to canvas spends a lot of time in URL getting ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-11-01 11:22 PDT by Jer Noble
Modified: 2017-11-02 08:43 PDT (History)
5 users (show)

See Also:


Attachments
Patch (15.82 KB, patch)
2017-11-01 11:27 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (15.82 KB, patch)
2017-11-01 14:23 PDT, Jer Noble
jer.noble: commit-queue+
Details | Formatted Diff | Diff
Patch for landing (15.81 KB, patch)
2017-11-01 14:25 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2017-11-01 11:22:10 PDT
[Performance] Painting <video> to canvas spends a lot of time in URL getting and parsing
Comment 1 Radar WebKit Bug Importer 2017-11-01 11:26:55 PDT
<rdar://problem/35297356>
Comment 2 Jer Noble 2017-11-01 11:27:45 PDT
Created attachment 325599 [details]
Patch
Comment 3 Build Bot 2017-11-01 11:30:02 PDT
Attachment 325599 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:208:  The parameter name "url" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Eric Carlson 2017-11-01 12:49:22 PDT
Comment on attachment 325599 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=325599&action=review

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:208
> +    void createAVAssetForURL(const URL& url) override;

Nit: "url" isn't necessary.
Comment 5 Jer Noble 2017-11-01 14:23:44 PDT
Created attachment 325630 [details]
Patch for landing
Comment 6 Jer Noble 2017-11-01 14:25:39 PDT
Created attachment 325631 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2017-11-01 14:42:02 PDT
Comment on attachment 325631 [details]
Patch for landing

Clearing flags on attachment: 325631

Committed r224297: <https://trac.webkit.org/changeset/224297>
Comment 8 WebKit Commit Bot 2017-11-01 14:42:04 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Eric Carlson 2017-11-01 17:30:55 PDT
(In reply to Ryan Haddad from comment #9)
> This change broke the Windows build:
> https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/
> 5727/steps/compile-webkit/logs/stdio
> 
> Here is the log after an attempt to fix it in
> https://trac.webkit.org/changeset/224305/webkit:
> https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/
> 5732/steps/compile-webkit/logs/stdio

Looks like you forgot to update MediaPlayerPrivateAVFoundationCF::createAVAssetForURL for the signature change.
Comment 11 Ryan Haddad 2017-11-02 08:43:27 PDT
The build was fixed with https://trac.webkit.org/changeset/224311/webkit