[Media Router] Increment the media's current_time in the WebUI route controller

[Media Router] Increment the media's current_time in the WebUI route controller

This CL replaces the is_paused attribute of the MediaRoute object with play_state
enum, and makes the WebUI route controller increment the current time of
the media every second if the enum value is PLAYING (not PAUSED/BUFFERING).

This is the second of the three (extension-side, browser-side, extension-side)
patches to allow the WebUI route controller to increment the current time (shown
on the seek bar) on its own while the media is playing:

1. Add play_state attribute to the extension side (cl/159493444)
2. Make the browser side use only play_state, and increment time on its own
(this patch)
3. Remove is_paused attribute and time-incrementing-logic from the extension
side

This CL must land after the CL #1 above.

BUG=731192
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2932933002
Cr-Commit-Position: refs/heads/master@{#481318}
Committed: https://chromium.googlesource.com/chromium/src/+/572110ec256c5a328a0cc6f17a893c27418410fb

[takumif, 2017-06-09 23:55:02]

Description was changed from

==========
[Media Router] Replace MediaStatus::is_paused attribute with play_state enum

BUG=731192
==========

to

==========
[Media Router] Replace MediaStatus::is_paused attribute with play_state enum

BUG=731192
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[takumif, 2017-06-20 01:38:12]

Description was changed from

==========
[Media Router] Replace MediaStatus::is_paused attribute with play_state enum

BUG=731192
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[Media Router] Increment the media's current_time in the WebUI route controller

This CL replaces the is_paused attribute with play_state enum, and makes the
WebUI route controller increment the current time of the media every second if
the enum value is PLAYING (not PAUSED/BUFFERING).

This is the second of the three (extension-side, browser-side, extension-side)
patches to allow the WebUI route controller to increment the current time (shown
on the seek bar) on its own while the media is playing:

1. Add play_state attribute to the extension side (cl/159493444)
2. Make the browser side use only play_state, and increment time on its own
(this patch)
3. Remove is_paused attribute and time-incrementing-logic from the extension
side

This CL must land after the CL #1 above.

BUG=731192
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[takumif, 2017-06-20 01:38:23]

Description was changed from

==========
[Media Router] Increment the media's current_time in the WebUI route controller

This CL replaces the is_paused attribute with play_state enum, and makes the
WebUI route controller increment the current time of the media every second if
the enum value is PLAYING (not PAUSED/BUFFERING).

This is the second of the three (extension-side, browser-side, extension-side)
patches to allow the WebUI route controller to increment the current time (shown
on the seek bar) on its own while the media is playing:

1. Add play_state attribute to the extension side (cl/159493444)
2. Make the browser side use only play_state, and increment time on its own
(this patch)
3. Remove is_paused attribute and time-incrementing-logic from the extension
side

This CL must land after the CL #1 above.

BUG=731192
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[Media Router] Increment the media's current_time in the WebUI route controller

This CL replaces the is_paused attribute with play_state enum, and makes the
WebUI route controller increment the current time of the media every second if
the enum value is PLAYING (not PAUSED/BUFFERING).

This is the second of the three (extension-side, browser-side, extension-side)
patches to allow the WebUI route controller to increment the current time (shown
on the seek bar) on its own while the media is playing:

1. Add play_state attribute to the extension side (cl/159493444)
2. Make the browser side use only play_state, and increment time on its own
(this patch)
3. Remove is_paused attribute and time-incrementing-logic from the extension
side

This CL must land after the CL #1 above.

BUG=731192
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[takumif, 2017-06-20 02:45:39]

Description was changed from

==========
[Media Router] Increment the media's current_time in the WebUI route controller

This CL replaces the is_paused attribute with play_state enum, and makes the
WebUI route controller increment the current time of the media every second if
the enum value is PLAYING (not PAUSED/BUFFERING).

This is the second of the three (extension-side, browser-side, extension-side)
patches to allow the WebUI route controller to increment the current time (shown
on the seek bar) on its own while the media is playing:

1. Add play_state attribute to the extension side (cl/159493444)
2. Make the browser side use only play_state, and increment time on its own
(this patch)
3. Remove is_paused attribute and time-incrementing-logic from the extension
side

This CL must land after the CL #1 above.

BUG=731192
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[Media Router] Increment the media's current_time in the WebUI route controller

This CL replaces the is_paused attribute of the MediaRoute object with play
state enum, and makes the WebUI route controller increment the current time of
the media every second if the enum value is PLAYING (not PAUSED/BUFFERING).

This is the second of the three (extension-side, browser-side, extension-side)
patches to allow the WebUI route controller to increment the current time (shown
on the seek bar) on its own while the media is playing:

1. Add play_state attribute to the extension side (cl/159493444)
2. Make the browser side use only play_state, and increment time on its own
(this patch)
3. Remove is_paused attribute and time-incrementing-logic from the extension
side

This CL must land after the CL #1 above.

BUG=731192
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[takumif, 2017-06-20 02:45:40]

takumif@chromium.org changed reviewers:
+ dcheng@chromium.org, imcheng@chromium.org

[takumif, 2017-06-20 16:55:16]

Description was changed from

==========
[Media Router] Increment the media's current_time in the WebUI route controller

This CL replaces the is_paused attribute of the MediaRoute object with play
state enum, and makes the WebUI route controller increment the current time of
the media every second if the enum value is PLAYING (not PAUSED/BUFFERING).

This is the second of the three (extension-side, browser-side, extension-side)
patches to allow the WebUI route controller to increment the current time (shown
on the seek bar) on its own while the media is playing:

1. Add play_state attribute to the extension side (cl/159493444)
2. Make the browser side use only play_state, and increment time on its own
(this patch)
3. Remove is_paused attribute and time-incrementing-logic from the extension
side

This CL must land after the CL #1 above.

BUG=731192
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[Media Router] Increment the media's current_time in the WebUI route controller

This CL replaces the is_paused attribute of the MediaRoute object with
play_state enum, and makes the WebUI route controller increment the current time
of
the media every second if the enum value is PLAYING (not PAUSED/BUFFERING).

This is the second of the three (extension-side, browser-side, extension-side)
patches to allow the WebUI route controller to increment the current time (shown
on the seek bar) on its own while the media is playing:

1. Add play_state attribute to the extension side (cl/159493444)
2. Make the browser side use only play_state, and increment time on its own
(this patch)
3. Remove is_paused attribute and time-incrementing-logic from the extension
side

This CL must land after the CL #1 above.

BUG=731192
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[takumif, 2017-06-20 16:55:36]

Description was changed from

==========
[Media Router] Increment the media's current_time in the WebUI route controller

This CL replaces the is_paused attribute of the MediaRoute object with
play_state enum, and makes the WebUI route controller increment the current time
of
the media every second if the enum value is PLAYING (not PAUSED/BUFFERING).

This is the second of the three (extension-side, browser-side, extension-side)
patches to allow the WebUI route controller to increment the current time (shown
on the seek bar) on its own while the media is playing:

1. Add play_state attribute to the extension side (cl/159493444)
2. Make the browser side use only play_state, and increment time on its own
(this patch)
3. Remove is_paused attribute and time-incrementing-logic from the extension
side

This CL must land after the CL #1 above.

BUG=731192
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[Media Router] Increment the media's current_time in the WebUI route controller

This CL replaces the is_paused attribute of the MediaRoute object with
play_state
enum, and makes the WebUI route controller increment the current time of
the media every second if the enum value is PLAYING (not PAUSED/BUFFERING).

This is the second of the three (extension-side, browser-side, extension-side)
patches to allow the WebUI route controller to increment the current time (shown
on the seek bar) on its own while the media is playing:

1. Add play_state attribute to the extension side (cl/159493444)
2. Make the browser side use only play_state, and increment time on its own
(this patch)
3. Remove is_paused attribute and time-incrementing-logic from the extension
side

This CL must land after the CL #1 above.

BUG=731192
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[takumif, 2017-06-20 17:19:25]

Please take a look at:
imcheng: all
dcheng: media_status.mojom and struct traits

Thanks!

[dcheng, 2017-06-20 21:18:05]

ipc lgtm

https://codereview.chromium.org/2932933002/diff/80001/chrome/common/media_rou...
File chrome/common/media_router/mojo/media_status_struct_traits.h (right):

https://codereview.chromium.org/2932933002/diff/80001/chrome/common/media_rou...
chrome/common/media_router/mojo/media_status_struct_traits.h:46: return false;
For enum traits, it's actually OK to NOTREACHED() this as well.

mojo enums basically go through double validation... =(

[imcheng, 2017-06-20 23:00:00]

lgtm % comments

https://codereview.chromium.org/2932933002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/media_router/elements/route_controls/route_controls.js
(right):

https://codereview.chromium.org/2932933002/diff/80001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_controls/route_controls.js:88:
'', '', false, false, false, false, 0, false, 0, 0, 0),
nit: Can we have a default constructor for RouteStatus?

https://codereview.chromium.org/2932933002/diff/80001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_controls/route_controls.js:93:
* media. Is 0 when the current time is not being incremented.
, or 0 if the current time is not being incremented.

https://codereview.chromium.org/2932933002/diff/80001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_controls/route_controls.js:251:
if (!this.timeIncrementsTimeoutId_) {
Do we need to check this.canIncrementCurrentTime_ first? Also, if the state
becomes BUFFERED or PAUSED and there is a timeout ID, we would need to clear it.

https://codereview.chromium.org/2932933002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/media_router/media_router_data.js (right):

https://codereview.chromium.org/2932933002/diff/80001/chrome/browser/resource...
chrome/browser/resources/media_router/media_router_data.js:275: /** @type
{!media_router.PlayState} */
! not needed

[takumif, 2017-06-20 23:56:12]

The CQ bit was checked by takumif@chromium.org to run a CQ dry run

[takumif, 2017-06-20 23:56:31]

Thanks for the reviews!

https://codereview.chromium.org/2932933002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/media_router/elements/route_controls/route_controls.js
(right):

https://codereview.chromium.org/2932933002/diff/80001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_controls/route_controls.js:88:
'', '', false, false, false, false, 0, false, 0, 0, 0),
On 2017/06/20 23:00:00, imcheng wrote:
> nit: Can we have a default constructor for RouteStatus?

Added default argument values to the ctor parameters.

https://codereview.chromium.org/2932933002/diff/80001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_controls/route_controls.js:93:
* media. Is 0 when the current time is not being incremented.
On 2017/06/20 23:00:00, imcheng wrote:
> , or 0 if the current time is not being incremented.

Done.

https://codereview.chromium.org/2932933002/diff/80001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_controls/route_controls.js:251:
if (!this.timeIncrementsTimeoutId_) {
On 2017/06/20 23:00:00, imcheng wrote:
> Do we need to check this.canIncrementCurrentTime_ first?
We could, but it's not necessary, since maybeIncrementCurrentTime_() checks
anyways.

> Also, if the state
> becomes BUFFERED or PAUSED and there is a timeout ID, we would need to clear
it.
maybeIncrementCurrentTime_() clears the timeout ID if the state is no longer
PLAYING, so I don't think we need to do that here.

https://codereview.chromium.org/2932933002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/media_router/media_router_data.js (right):

https://codereview.chromium.org/2932933002/diff/80001/chrome/browser/resource...
chrome/browser/resources/media_router/media_router_data.js:275: /** @type
{!media_router.PlayState} */
On 2017/06/20 23:00:00, imcheng wrote:
> ! not needed

Done.

https://codereview.chromium.org/2932933002/diff/80001/chrome/common/media_rou...
File chrome/common/media_router/mojo/media_status_struct_traits.h (right):

https://codereview.chromium.org/2932933002/diff/80001/chrome/common/media_rou...
chrome/common/media_router/mojo/media_status_struct_traits.h:46: return false;
On 2017/06/20 21:18:05, dcheng wrote:
> For enum traits, it's actually OK to NOTREACHED() this as well.
> 
> mojo enums basically go through double validation... =(

Done.

[commit-bot: I haz the power, 2017-06-20 23:56:37]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[takumif, 2017-06-21 17:17:10]

The CQ bit was checked by takumif@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-21 17:17:29]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-21 17:58:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-21 17:59:00]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[takumif, 2017-06-21 18:18:24]

The CQ bit was checked by takumif@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-21 18:18:38]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-21 20:27:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-21 20:27:47]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[takumif, 2017-06-21 20:32:23]

The CQ bit was checked by takumif@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-21 20:32:46]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-21 21:43:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-21 21:43:25]

Dry run: This issue passed the CQ dry run.

[takumif, 2017-06-21 21:44:17]

The CQ bit was checked by takumif@chromium.org

[takumif, 2017-06-21 21:44:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
imcheng@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2932933002/#ps120001
(title: "Check that we can increment in onRouteStatusChange_()")

[commit-bot: I haz the power, 2017-06-21 21:44:40]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-21 21:49:15]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1498081457737930,
"parent_rev": "e8c5ee20d4d2798573896fa1a0c78d58349d3286", "commit_rev":
"271839688e8c1737048848bf12f384406203a359"}

[commit-bot: I haz the power, 2017-06-21 21:49:42]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1498081457737930,
"parent_rev": "5c4bbafa0d3cee8b39639b62e3990832a3df7522", "commit_rev":
"e2545e0a3bd3beca97dad603d092b6eae665d3bb"}

[commit-bot: I haz the power, 2017-06-21 21:49:58]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1498081457737930,
"parent_rev": "1ff035c227597955e30e484dbeb6d059ddb13326", "commit_rev":
"572110ec256c5a328a0cc6f17a893c27418410fb"}

[commit-bot: I haz the power, 2017-06-21 21:50:13]

Description was changed from

==========
[Media Router] Increment the media's current_time in the WebUI route controller

This CL replaces the is_paused attribute of the MediaRoute object with
play_state
enum, and makes the WebUI route controller increment the current time of
the media every second if the enum value is PLAYING (not PAUSED/BUFFERING).

This is the second of the three (extension-side, browser-side, extension-side)
patches to allow the WebUI route controller to increment the current time (shown
on the seek bar) on its own while the media is playing:

1. Add play_state attribute to the extension side (cl/159493444)
2. Make the browser side use only play_state, and increment time on its own
(this patch)
3. Remove is_paused attribute and time-incrementing-logic from the extension
side

This CL must land after the CL #1 above.

BUG=731192
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[Media Router] Increment the media's current_time in the WebUI route controller

This CL replaces the is_paused attribute of the MediaRoute object with
play_state
enum, and makes the WebUI route controller increment the current time of
the media every second if the enum value is PLAYING (not PAUSED/BUFFERING).

This is the second of the three (extension-side, browser-side, extension-side)
patches to allow the WebUI route controller to increment the current time (shown
on the seek bar) on its own while the media is playing:

1. Add play_state attribute to the extension side (cl/159493444)
2. Make the browser side use only play_state, and increment time on its own
(this patch)
3. Remove is_paused attribute and time-incrementing-logic from the extension
side

This CL must land after the CL #1 above.

BUG=731192
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2932933002
Cr-Commit-Position: refs/heads/master@{#481318}
Committed:
https://chromium.googlesource.com/chromium/src/+/572110ec256c5a328a0cc6f17a89...
==========

[commit-bot: I haz the power, 2017-06-21 21:50:15]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/572110ec256c5a328a0cc6f17a89...