Bug 169156 – [iOS] The web process should inherit application state from UI process

• Component: Media
• Status: RESOLVED FIXED

[Eric Carlson]

The web process should inherit application state from UI process

[Radar WebKit Bug Importer]

<rdar://problem/30845473>

[Eric Carlson]

Created attachment 303394 [details]
Proposed patch.

[Jon Lee]

Comment on attachment 303394 [details]
Proposed patch.

R=me but needs WK2 reviewer.

[Alexey Proskuryakov]

Comment on attachment 303394 [details]
Proposed patch.

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

Only a shallow review, as I don't know about the overall design.

> Source/WebKit2/Platform/spi/ios/CelestialSPI.h:25
> +

#pragma once

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:118
> +SOFT_LINK_CONSTANT_MAY_FAIL(Celestial, AVSystemController_PIDToInheritApplicationStateFrom, NSString*)

Misplaced star.

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:135
> +        NSError* error;

Please initialize the variable to nil. Also, misplaced star.

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:138
> +            WTFLogAlways("Failed to set up PID proxying");

It may be useful to log the error.

[Eric Carlson]

Comment on attachment 303394 [details]
Proposed patch.

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

>> Source/WebKit2/Platform/spi/ios/CelestialSPI.h:25
>> +
> 
> #pragma once

Fixed.

>> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:118
>> +SOFT_LINK_CONSTANT_MAY_FAIL(Celestial, AVSystemController_PIDToInheritApplicationStateFrom, NSString*)
> 
> Misplaced star.

Fixed.

>> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:135
>> +        NSError* error;
> 
> Please initialize the variable to nil. Also, misplaced star.

Fixed.

>> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:138
>> +            WTFLogAlways("Failed to set up PID proxying");
> 
> It may be useful to log the error.

Good idea, fixed.

[Eric Carlson]

Created attachment 303406 [details]
Updated patch.

[WebKit Commit Bot]

Comment on attachment 303406 [details]
Updated patch.

Clearing flags on attachment: 303406

Committed r213430: <http://trac.webkit.org/changeset/213430>

Comment on attachment 303406 [details]
Updated patch.

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

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:139
> +    if (canLoadAVSystemController_PIDToInheritApplicationStateFrom()) {
> +        pid_t pid = WebProcess::singleton().presenterApplicationPid();
> +        NSError *error = nil;
> +        [[getAVSystemControllerClass() sharedAVSystemController] setAttribute:@(pid) forKey:AVSystemController_PIDToInheritApplicationStateFrom error:&error];
> +        if (error)
> +            WTFLogAlways("Failed to set up PID proxying: %s", [[error localizedDescription] UTF8String]);
> +    }

Why are we doing global, pre-process setup in WebPage initialization rather than in web process initialization?

[Eric Carlson]

Created attachment 304384 [details]
Patch for landing.

Address Dan's comments.

[Brady Eidson]

Comment on attachment 304384 [details]
Patch for landing.

After bots are happy (they appear to not be!)

[Eric Carlson]

Created attachment 304389 [details]
Patch for landing.

[WebKit Commit Bot]

Comment on attachment 304389 [details]
Patch for landing.

Clearing flags on attachment: 304389

Committed r213933: <http://trac.webkit.org/changeset/213933>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.