Update ScreenTime as playback state changes
<rdar://problem/61181092>
Created attachment 396481 [details] WIP patch for the bots to check
Created attachment 396497 [details] updated WIP patch for the bots
Created attachment 396528 [details] updated WIP patch for the bots
Created attachment 396540 [details] updated WIP patch for the bots
Comment on attachment 396540 [details] updated WIP patch for the bots View in context: https://bugs.webkit.org/attachment.cgi?id=396540&action=review > Source/WebCore/ChangeLog:1 > +2020-04-14 Eric Carlson <eric.carlson@apple.com> ... > Source/WebCore/ChangeLog:60 > +2020-04-14 Eric Carlson <eric.carlson@apple.com> Looks like you got a double changelong here. > Source/WebKit/ChangeLog:52 > +2020-04-14 Eric Carlson <eric.carlson@apple.com> Here too. > Source/WebCore/html/MediaElementSession.cpp:123 > +#if PLATFORM(MACCATALYST) || PLATFORM(MAC) Could you add a ENABLE(MEDIA_USAGE) pragma and use it in platform-independent code instead? That way other ports could adopt without having to touch all these call sites. > Source/WebKit/UIProcess/WebPageProxy.h:1739 > + void addMediaUsageManagerSession(WebCore::MediaSessionIdentifier, const String& /* bundleIdentifier */, const URL& /* pageURL */); Is there a reason these parameter names are commented out?
Created attachment 396586 [details] Patch
Comment on attachment 396540 [details] updated WIP patch for the bots View in context: https://bugs.webkit.org/attachment.cgi?id=396540&action=review >> Source/WebCore/ChangeLog:60 >> +2020-04-14 Eric Carlson <eric.carlson@apple.com> > > Looks like you got a double changelong here. Fixed. >> Source/WebCore/html/MediaElementSession.cpp:123 >> +#if PLATFORM(MACCATALYST) || PLATFORM(MAC) > > Could you add a ENABLE(MEDIA_USAGE) pragma and use it in platform-independent code instead? That way other ports could adopt without having to touch all these call sites. Good suggestion, changed. >> Source/WebKit/UIProcess/WebPageProxy.h:1739 >> + void addMediaUsageManagerSession(WebCore::MediaSessionIdentifier, const String& /* bundleIdentifier */, const URL& /* pageURL */); > > Is there a reason these parameter names are commented out? Left over from an earlier version with more parameters. Removed.
Comment on attachment 396586 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=396586&action=review > Source/WebKit/UIProcess/Media/cocoa/MediaUsageManagerCocoa.h:42 > + void reset() final; Nit. Looks like these functions can be private?
Comment on attachment 396586 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=396586&action=review > Source/WebCore/html/MediaElementSession.cpp:1021 > + auto* fullscreenElement = m_element.document().fullscreenManager().currentFullscreenElement(); document.fullscreenManager()... > Source/WebCore/html/MediaElementSession.cpp:1036 > + m_element.document().isMediaDocument() && (m_element.document().frame() && m_element.document().frame()->isMainFrame()), ditto
Comment on attachment 396586 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=396586&action=review R=me with just a couple of nits. > Source/WebCore/testing/Internals.idl:120 > +[ > + ExportMacro=WEBCORE_TESTSUPPORT_EXPORT, > + Conditional=VIDEO, > + JSGenerateToJSObject, > +] dictionary MediaUsageState { It's unfortunate you can't just re-use MediaUsageInfo here, and not re-declare the existing struct, but I guess that the URL -> DOMString for mediaURL makes that untenable. > Source/WebKit/UIProcess/Media/cocoa/MediaUsageManagerCocoa.h:63 > + WebCore::MediaSessionIdentifier m_identifier; > + String m_bundleIdentifier; > + URL m_pageURL; > + Optional<WebCore::MediaUsageInfo> m_mediaUsageInfo; > + RetainPtr<USVideoUsage> m_usageTracker; Since structs have public members, there's no no need to prefix these with `m_`. > Source/WebKit/UIProcess/Media/cocoa/MediaUsageManagerCocoa.mm:198 > + WTFLogAlways("MediaUsageManagerCocoa::updateMediaUsage caught exception!"); Do you want to include the exception.description here?
Created attachment 396616 [details] Patch for landing
Committed r260182: <https://trac.webkit.org/changeset/260182> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396616 [details].
Reopening to attach new patch.
Created attachment 396672 [details] Follow-up Patch
Committed r260201: <https://trac.webkit.org/changeset/260201>