Bug 208727

Summary: [GPUP] Move AVSystemController code into the GPU process
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, ews-watchlist, glenn, jonlee, peng.liu6, philipj, sergio, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
eric.carlson: review+
Patch for landing
none
Patch for landing
none
Patch for landing
none
Patch for landing jer.noble: commit-queue+

Description Jer Noble 2020-03-06 11:42:17 PST 
[GPUP] Move AVSystemController code into the GPU process
Comment 1 Jer Noble 2020-03-06 12:10:17 PST 
Created attachment 392750 [details]
Patch
Comment 2 Jer Noble 2020-03-06 12:17:51 PST 
Created attachment 392752 [details]
Patch
Comment 3 Jer Noble 2020-03-06 12:35:36 PST 
Created attachment 392755 [details]
Patch
Comment 4 Eric Carlson 2020-03-06 15:54:04 PST 
Comment on attachment 392755 [details]
Patch

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

> Source/WebCore/platform/audio/ios/MediaSessionHelperIOS.mm:72
> +#if HAVE(MEDIA_PLAYER) && !PLATFORM(WATCHOS)

HAVE_MEDIA_PLAYER is always defined for PLATFORM(COCOA)

> Source/WebCore/platform/audio/ios/MediaSessionHelperIOS.mm:88
> +#if HAVE(MEDIA_PLAYER) && !PLATFORM(WATCHOS)

Ditto.

> Source/WebCore/platform/audio/ios/MediaSessionHelperIOS.mm:183
> +    m_objcObserver = nil;

Nit: this isn't necessary since it is a RetainPtr

> Source/WebCore/platform/audio/ios/MediaSessionHelperIOS.mm:361
> +#if HAVE(MEDIA_PLAYER) && !PLATFORM(WATCHOS)

HAVE_MEDIA_PLAYER is always defined for PLATFORM(COCOA)

> Source/WebCore/platform/audio/ios/MediaSessionHelperIOS.mm:387
> +#if HAVE(MEDIA_PLAYER) && !PLATFORM(WATCHOS)

Ditto
Comment 5 Jer Noble 2020-03-06 16:32:34 PST 
Created attachment 392803 [details]
Patch for landing
Comment 6 Radar WebKit Bug Importer 2020-03-06 18:01:59 PST 
<rdar://problem/60178482>
Comment 7 Jer Noble 2020-03-07 08:41:02 PST 
Won't land until I fix those test failures.
Comment 8 Jer Noble 2020-03-07 08:49:44 PST 
Created attachment 392860 [details]
Patch for landing
Comment 9 Jer Noble 2020-03-08 10:13:41 PDT 
Created attachment 392958 [details]
Patch for landing
Comment 10 Jer Noble 2020-03-08 10:57:16 PDT 
Created attachment 392961 [details]
Patch for landing
Comment 11 Jer Noble 2020-03-08 12:04:56 PDT 
Layout test failure (fast/hidpi/image-srcset-relative-svg-canvas-2x.html) is a failure already in the tree (image diff by 0.01%), and is unrelated. Landing.
Comment 12 Jer Noble 2020-03-08 12:22:48 PDT 
Committed r258109: <https://trac.webkit.org/changeset/258109>
Comment 13 youenn fablet 2020-03-09 02:34:49 PDT 
We register two applicationWillEnterForeground for get_UIKit_UIApplicationWillEnterForegroundNotification and WebUIApplicationWillEnterForegroundNotification.
Ditto for applicationDidBecomeActive, applicationWillResignActive and applicationDidEnterBackground.

Given we post a WebUIApplicationWillEnterForegroundNotification notification from WebProcess/WebPage, shouldn't we listen to this notification from WebProcess only?

Maybe we do not need WebUIApplicationWillEnterForegroundNotification anymore and can rely on get_UIKit_UIApplicationWillEnterForegroundNotification only? Ditto for others?

Also, we are listening for UIKit_UIApplicationWillEnterForegroundNotification in both UIProcess and GPUProcess. Do we need both? If we need both, should we try to have a single code path (GPUProcess -> UIProcess -> WebProcess for instance)?