Bug 168985 - [iOS] Disable autoplay of silent videos in low power mode
Summary: [iOS] Disable autoplay of silent videos in low power mode
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on: 168837
Blocks:
  Show dependency treegraph
 
Reported: 2017-02-28 12:54 PST by Chris Dumez
Modified: 2017-03-06 11:43 PST (History)
11 users (show)

See Also:


Attachments
WIP Patch (3.01 KB, patch)
2017-02-28 13:49 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (3.01 KB, patch)
2017-02-28 14:46 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (3.78 KB, patch)
2017-03-01 09:07 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (7.90 KB, patch)
2017-03-01 09:32 PST, Chris Dumez
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews123 for ios-simulator-wk2 (deleted)
2017-03-01 11:04 PST, Build Bot
no flags Details
WIP Patch (5.88 KB, patch)
2017-03-01 11:18 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (7.51 KB, patch)
2017-03-01 11:21 PST, Chris Dumez
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-elcapitan (1.09 MB, application/zip)
2017-03-01 12:32 PST, Build Bot
no flags Details
Archive of layout-test-results from ews112 for mac-elcapitan (1.76 MB, application/zip)
2017-03-01 12:47 PST, Build Bot
no flags Details
WIP Patch (8.34 KB, patch)
2017-03-01 12:55 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (8.71 KB, patch)
2017-03-01 13:11 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (8.71 KB, patch)
2017-03-01 13:50 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (16.42 KB, patch)
2017-03-02 09:45 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-elcapitan (896.26 KB, application/zip)
2017-03-02 10:50 PST, Build Bot
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (13.09 MB, application/zip)
2017-03-02 11:10 PST, Build Bot
no flags Details
Patch (17.22 KB, patch)
2017-03-02 12:15 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2017-02-28 12:54:21 PST
Disable autoplay of silent videos in low power mode on iOS to save battery.
Comment 1 Chris Dumez 2017-02-28 12:54:59 PST
<rdar://problem/30739051>
Comment 2 Chris Dumez 2017-02-28 13:49:00 PST
Created attachment 302979 [details]
WIP Patch
Comment 3 Chris Dumez 2017-02-28 14:46:36 PST
Created attachment 302989 [details]
WIP Patch
Comment 4 Chris Dumez 2017-03-01 09:07:07 PST
Created attachment 303068 [details]
WIP Patch
Comment 5 Chris Dumez 2017-03-01 09:32:25 PST
Created attachment 303071 [details]
WIP Patch
Comment 6 WebKit Commit Bot 2017-03-01 09:33:35 PST
Attachment 303071 [details] did not pass style-queue:


ERROR: LayoutTests/platform/ios-simulator/TestExpectations:2734:  Path does not exist.  [test/expectations] [5]
ERROR: LayoutTests/platform/ios-simulator/TestExpectations:2735:  Path does not exist.  [test/expectations] [5]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Build Bot 2017-03-01 11:04:16 PST
Comment on attachment 303071 [details]
WIP Patch

Attachment 303071 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3216665

New failing tests:
media/video-autoplay-lowPowerMode.html
media/video-autoplay-lowPowerMode-with-controls.html
Comment 8 Build Bot 2017-03-01 11:04:22 PST
Created attachment 303082 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 9 Chris Dumez 2017-03-01 11:18:18 PST
Created attachment 303084 [details]
WIP Patch
Comment 10 Chris Dumez 2017-03-01 11:21:13 PST
Created attachment 303085 [details]
WIP Patch
Comment 11 Build Bot 2017-03-01 12:32:21 PST
Comment on attachment 303085 [details]
WIP Patch

Attachment 303085 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3217203

New failing tests:
http/tests/media/modern-media-controls/skip-back-support/skip-back-support-live-broadcast.html
media/modern-media-controls/media-controller/media-controller-compact.html
media/modern-media-controls/volume-support/volume-support-drag.html
media/modern-media-controls/scrubber-support/scrubber-support-click.html
media/modern-media-controls/fullscreen-support/fullscreen-support-click.html
media/modern-media-controls/playback-support/playback-support-button-click.html
media/modern-media-controls/start-support/start-support-autoplay.html
media/modern-media-controls/playback-support/playback-support-autoplay.html
media/modern-media-controls/status-support/status-support-playing.html
media/modern-media-controls/scrubber-support/scrubber-support-drag.html
media/modern-media-controls/volume-support/volume-support-click.html
http/tests/media/modern-media-controls/status-support/status-support-live-broadcast.html
Comment 12 Build Bot 2017-03-01 12:32:26 PST
Created attachment 303096 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 13 Build Bot 2017-03-01 12:47:25 PST
Comment on attachment 303085 [details]
WIP Patch

Attachment 303085 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3217253

New failing tests:
http/tests/media/modern-media-controls/skip-back-support/skip-back-support-live-broadcast.html
media/modern-media-controls/media-controller/media-controller-compact.html
media/modern-media-controls/volume-support/volume-support-drag.html
media/modern-media-controls/scrubber-support/scrubber-support-click.html
media/modern-media-controls/fullscreen-support/fullscreen-support-click.html
media/modern-media-controls/playback-support/playback-support-button-click.html
media/modern-media-controls/volume-support/volume-support-click.html
media/modern-media-controls/playback-support/playback-support-autoplay.html
media/modern-media-controls/status-support/status-support-playing.html
media/modern-media-controls/scrubber-support/scrubber-support-drag.html
media/modern-media-controls/start-support/start-support-autoplay.html
http/tests/media/modern-media-controls/status-support/status-support-live-broadcast.html
Comment 14 Build Bot 2017-03-01 12:47:31 PST
Created attachment 303098 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 15 Chris Dumez 2017-03-01 12:55:57 PST
Created attachment 303101 [details]
WIP Patch
Comment 16 Chris Dumez 2017-03-01 13:09:00 PST
Comment on attachment 303101 [details]
WIP Patch

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

> Source/WebCore/Modules/mediacontrols/MediaControlsHost.idl:51
> +    readonly attribute boolean shouldForceControlsDisplay;

Weird:
CONSOLE MESSAGE: line 66: TypeError: null is not an object (evaluating 'this.mediaController.host.shouldForceControlsDisplay') in some layout tests.
Comment 17 Chris Dumez 2017-03-01 13:11:23 PST
Created attachment 303105 [details]
WIP Patch
Comment 18 Chris Dumez 2017-03-01 13:12:00 PST
(In reply to comment #16)
> Comment on attachment 303101 [details]
> WIP Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=303101&action=review
> 
> > Source/WebCore/Modules/mediacontrols/MediaControlsHost.idl:51
> > +    readonly attribute boolean shouldForceControlsDisplay;
> 
> Weird:
> CONSOLE MESSAGE: line 66: TypeError: null is not an object (evaluating
> 'this.mediaController.host.shouldForceControlsDisplay') in some layout tests.

It seems this.mediaController.host can be null. Other places using it have null checks so I'll do the same.
Comment 19 Antoine Quint 2017-03-01 13:26:39 PST
(In reply to comment #18)
> (In reply to comment #16)
> > Comment on attachment 303101 [details]
> > WIP Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=303101&action=review
> > 
> > > Source/WebCore/Modules/mediacontrols/MediaControlsHost.idl:51
> > > +    readonly attribute boolean shouldForceControlsDisplay;
> > 
> > Weird:
> > CONSOLE MESSAGE: line 66: TypeError: null is not an object (evaluating
> > 'this.mediaController.host.shouldForceControlsDisplay') in some layout tests.
> 
> It seems this.mediaController.host can be null. Other places using it have
> null checks so I'll do the same.

That's right, you need to expect the MediaControlsHost may be null, some tests do not use media elements at all and pass in a null host to the MediaController constructor.
Comment 20 Chris Dumez 2017-03-01 13:50:17 PST
Created attachment 303112 [details]
WIP Patch
Comment 21 Chris Dumez 2017-03-02 09:45:48 PST
Created attachment 303207 [details]
Patch
Comment 22 Antoine Quint 2017-03-02 09:55:31 PST
Comment on attachment 303207 [details]
Patch

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

JS part looks good.

> Source/WebCore/Modules/modern-media-controls/media/controls-visibility-support.js:67
> +        const shouldShowControls = !!(media.controls || (host && host.shouldForceControlsDisplay));

Since the second part of the statement is guaranteed to return a boolean, I think you can simply do `!!media.controls || (host && host.shouldForceControlsDisplay)`
Comment 23 Chris Dumez 2017-03-02 10:00:23 PST
Comment on attachment 303207 [details]
Patch

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

>> Source/WebCore/Modules/modern-media-controls/media/controls-visibility-support.js:67
>> +        const shouldShowControls = !!(media.controls || (host && host.shouldForceControlsDisplay));
> 
> Since the second part of the statement is guaranteed to return a boolean, I think you can simply do `!!media.controls || (host && host.shouldForceControlsDisplay)`

FYI, I tried but it is not work:
FAIL mediaController.controls.controlsBar.visible should be false (of type boolean). Was null (of type object).

Code looked like:
const shouldShowControls = (!!media.controls || (host && host.shouldForceControlsDisplay));
Comment 24 Chris Dumez 2017-03-02 10:00:59 PST
(In reply to comment #23)
> Comment on attachment 303207 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=303207&action=review
> 
> >> Source/WebCore/Modules/modern-media-controls/media/controls-visibility-support.js:67
> >> +        const shouldShowControls = !!(media.controls || (host && host.shouldForceControlsDisplay));
> > 
> > Since the second part of the statement is guaranteed to return a boolean, I think you can simply do `!!media.controls || (host && host.shouldForceControlsDisplay)`
> 
> FYI, I tried but it is not work:
> FAIL mediaController.controls.controlsBar.visible should be false (of type
> boolean). Was null (of type object).
> 
> Code looked like:
> const shouldShowControls = (!!media.controls || (host &&
> host.shouldForceControlsDisplay));

*did* not work.
Comment 25 Chris Dumez 2017-03-02 10:04:08 PST
(In reply to comment #24)
> (In reply to comment #23)
> > Comment on attachment 303207 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=303207&action=review
> > 
> > >> Source/WebCore/Modules/modern-media-controls/media/controls-visibility-support.js:67
> > >> +        const shouldShowControls = !!(media.controls || (host && host.shouldForceControlsDisplay));
> > > 
> > > Since the second part of the statement is guaranteed to return a boolean, I think you can simply do `!!media.controls || (host && host.shouldForceControlsDisplay)`
> > 
> > FYI, I tried but it is not work:
> > FAIL mediaController.controls.controlsBar.visible should be false (of type
> > boolean). Was null (of type object).
> > 
> > Code looked like:
> > const shouldShowControls = (!!media.controls || (host &&
> > host.shouldForceControlsDisplay));
> 
> *did* not work.

It impacted tests like those:
media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-toggle.html
media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-off-audio.html
Comment 26 Build Bot 2017-03-02 10:50:25 PST
Comment on attachment 303207 [details]
Patch

Attachment 303207 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3223188

New failing tests:
media/modern-media-controls/start-support/start-support-lowPowerMode.html
Comment 27 Build Bot 2017-03-02 10:50:31 PST
Created attachment 303217 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 28 Build Bot 2017-03-02 11:10:34 PST
Comment on attachment 303207 [details]
Patch

Attachment 303207 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3223235

New failing tests:
media/modern-media-controls/start-support/start-support-lowPowerMode.html
Comment 29 Build Bot 2017-03-02 11:10:40 PST
Created attachment 303221 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 30 Chris Dumez 2017-03-02 12:15:02 PST
Created attachment 303223 [details]
Patch
Comment 31 WebKit Commit Bot 2017-03-06 11:43:30 PST
Comment on attachment 303223 [details]
Patch

Clearing flags on attachment: 303223

Committed r213460: <http://trac.webkit.org/changeset/213460>
Comment 32 WebKit Commit Bot 2017-03-06 11:43:40 PST
All reviewed patches have been landed.  Closing bug.