Adopt AVPlayerLayerView
Created attachment 256628 [details] Patch
Created attachment 256778 [details] Patch
Created attachment 256779 [details] Patch
rdar://problem/21661808
Need to add derivation of AVPictureInPicturePlayerLayerView and implement -pictureInPicturePlayerLayerView.
Created attachment 257085 [details] Patch
Attachment 257085 [details] did not pass style-queue: ERROR: Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:580: Should not have spaces around = in property attributes. [whitespace/property] [4] ERROR: Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:589: This { should be at the end of the previous line [whitespace/braces] [4] Total errors found: 2 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 257085 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257085&action=review > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:583 > +@property (nonatomic, copy, nullable) NSDictionary *pixelBufferAttributes; Is this necessary? I don't see where it's used in this patch. > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:750 > + static Class theClass = nil; > + if (!theClass) { > + theClass = objc_allocateClassPair(getAVPictureInPicturePlayerLayerViewClass(), "WebAVPictureInPicturePlayerLayerView", 0); If this method is ever called simultaneously on different threads (i.e., web thread vs. main thread), there's a possibility of a race condition. Perhaps dispatch_once() here instead? > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:848 > + static Class classWebAVPlayerLayerView = nil; > + if (!classWebAVPlayerLayerView) { > + classWebAVPlayerLayerView = objc_allocateClassPair(getAVPlayerLayerViewClass(), "WebAVPlayerLayerView", 0); Ditto.
(In reply to comment #8) > Comment on attachment 257085 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=257085&action=review > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:583 > > +@property (nonatomic, copy, nullable) NSDictionary *pixelBufferAttributes; > > Is this necessary? I don't see where it's used in this patch. It is necessary to look like an AVPlayerLayer, it is accessed by AVPlayerLayerView in AVKit. > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:750 > > + static Class theClass = nil; > > + if (!theClass) { > > + theClass = objc_allocateClassPair(getAVPictureInPicturePlayerLayerViewClass(), "WebAVPictureInPicturePlayerLayerView", 0); > > If this method is ever called simultaneously on different threads (i.e., web > thread vs. main thread), there's a possibility of a race condition. Perhaps > dispatch_once() here instead? WebVideoFullscreenInterfaceAVKit is only ever accessed from the main thread. All the jumping between threads happens in WebVideoFullscreenControllerAVKit. Yes, dispatch_once is the right way to initialize these. I'll change that. > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:848 > > + static Class classWebAVPlayerLayerView = nil; > > + if (!classWebAVPlayerLayerView) { > > + classWebAVPlayerLayerView = objc_allocateClassPair(getAVPlayerLayerViewClass(), "WebAVPlayerLayerView", 0); > > Ditto. Ditto.
Comment on attachment 257085 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257085&action=review > Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.mm:328 > + model->setLayerHostView(view.get()); Why the .get(), since setLayerHostView takes a RetainPtr<WebLayerHostView>&&?
Created attachment 257120 [details] Patch
(In reply to comment #10) > Comment on attachment 257085 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=257085&action=review > > > Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.mm:328 > > + model->setLayerHostView(view.get()); > > Why the .get(), since setLayerHostView takes a RetainPtr<WebLayerHostView>&&? I'll remove this.
Created attachment 257121 [details] Patch for landing.
Created attachment 257125 [details] Patch for landing.
Created attachment 257127 [details] Patch for landing.
Comment on attachment 257127 [details] Patch for landing. Rejecting attachment 257127 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 257127, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: rce/WebCore/platform/spi/cocoa/AVKitSPI.h patching file Source/WebCore/platform/spi/cocoa/QuartzCoreSPI.h Hunk #2 succeeded at 135 (offset 9 lines). patching file Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.h patching file Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.mm patching file Source/WebKit2/WebProcess/ios/WebVideoFullscreenManager.mm Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.appspot.com/results/5350159965749248
Created attachment 257131 [details] Patch for landing.
Comment on attachment 257131 [details] Patch for landing. Rejecting attachment 257131 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 257131, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: creenModelVideoElement.mm patching file Source/WebCore/platform/spi/cocoa/AVKitSPI.h patching file Source/WebCore/platform/spi/cocoa/QuartzCoreSPI.h patching file Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.h patching file Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.mm patching file Source/WebKit2/WebProcess/ios/WebVideoFullscreenManager.mm Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.appspot.com/results/5774197791916032
Created attachment 257134 [details] Patch for landing.
Comment on attachment 257134 [details] Patch for landing. Clearing flags on attachment: 257134 Committed r187044: <http://trac.webkit.org/changeset/187044>
All reviewed patches have been landed. Closing bug.