Bug 215861 – REGRESSION(r264710): Initializing the AVPlayer Obj-C class at process start up causes a regression in power-use tests

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

REGRESSION(r264710): Initializing the AVPlayer Obj-C class at process start up causes a regression in power-use tests

[Jer Noble]

<rdar://problem/67755363>

[Jer Noble]

Created attachment 407330 [details]
Patch

[Peng Liu]

Comment on attachment 407330 [details]
Patch

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

> Source/WebKit/ChangeLog:8
> +        Calling +instancesRespondToSelector: will cause the underyling Obj-C class to be initialize, which in the case of

Nit. s/initialize/initialized.

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1048
>      // and process global.

Nit. Maybe update the comment here as well?

[Jer Noble]

(In reply to Peng Liu from comment #3)
> Comment on attachment 407330 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=407330&action=review
> 
> > Source/WebKit/ChangeLog:8
> > +        Calling +instancesRespondToSelector: will cause the underyling Obj-C class to be initialize, which in the case of
> 
> Nit. s/initialize/initialized.

Fixed.

> > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1048
> >      // and process global.
> 
> Nit. Maybe update the comment here as well?

I think the comment still holds. We just assume that the property is there at runtime, rather than explicitly check.

[Jer Noble]

Created attachment 407334 [details]
Patch for landing

[Darin Adler]

Comment on attachment 407330 [details]
Patch

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

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1049
> +    return;

Given that this is now a compile time switch, I suggest wrapping the entire function body in:

#if !HAVE(AVPLAYER_VIDEORANGEOVERRIDE)

Rather than compiling a function with a return at the start of it.

[Jer Noble]

(In reply to Darin Adler from comment #6)
> Comment on attachment 407330 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=407330&action=review
> 
> > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1049
> > +    return;
> 
> Given that this is now a compile time switch, I suggest wrapping the entire
> function body in:
> 
> #if !HAVE(AVPLAYER_VIDEORANGEOVERRIDE)
> 
> Rather than compiling a function with a return at the start of it.

I considered that (or doing a #if/#else/#endif, but thought it might be confusing for previous OSs, especially given the existing comment. But sure, I'll re-write the comment to be more explicit that the wrapped behavior is for when this API is not available.

[Jer Noble]

Created attachment 407341 [details]
Patch for landing

[EWS]

Committed r266240: <https://trac.webkit.org/changeset/266240>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407341 [details].