Support to repeat one song in Audio Player.

Support to repeat one song in Audio Player.

Repeat mode can be 3 states.
A) Stop playing when all songs have been played.
B) Keep on playing by same order when all songs have been played.
C) Set a single song to repeat.

Created a repeat-button that has 1 state (A) as the OFF, and 2 states (B/C) as the ON in toggle-button in order to show all 3 states visually.

BUG=524884
TEST=browser_tests --gtest_filter=OpenAudioFiles*

Committed: https://crrev.com/ce5ebc11016280f6ef7b6bf69a06b9f5716caa75
Cr-Commit-Position: refs/heads/master@{#418755}

[harukam, 2016-09-02 11:27:26]

harukam@google.com changed reviewers:
+ fukino@chromium.org

[harukam, 2016-09-02 11:27:27]

Fukino-san, please take a look.

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
File ui/file_manager/audio_player/elements/audio_player.js (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/audio_player.js:283:
this.$.trackList.fire('current-track-index-changed');
Question, do you think this line is necessary?
According to the definition of 'currentTrackIndexChanged' in
'audio_player/elements/track_list.js' (observer of currentTrackIndex), it
doesn't seem do anything when the index doesn't change.

[fukino, 2016-09-02 12:11:49]

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
File ui/file_manager/audio_player/elements/audio_player.js (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/audio_player.js:274: advance_:
function(forward, repeatMode) {
I think we don't need to change this advance_ function.
Actually, in repeat-one mode, we don't advance to the next song.

Maybe we can handle the repeat-one case in scheduleAutoAdvance_.

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/audio_player.js:283:
this.$.trackList.fire('current-track-index-changed');
On 2016/09/02 11:27:27, harukam wrote:
> Question, do you think this line is necessary?
> According to the definition of 'currentTrackIndexChanged' in
> 'audio_player/elements/track_list.js' (observer of currentTrackIndex), it
> doesn't seem do anything when the index doesn't change.

According to https://codereview.chromium.org/1681843002,
the current-track-index-changed event is fired to restart playback when there is
only one song.

Please check if it works without the event firing when the mode is repeat-one
and there is only one song and the user press next/prev button.

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
File ui/file_manager/audio_player/elements/compiled_resources2.gyp (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/compiled_resources2.gyp:23: #     
'target_name': 'repeat_button',
Please sort targets based on their target names.

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
File ui/file_manager/audio_player/elements/control_panel.js (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/control_panel.js:83: repeat: {
This property should be repeatMode for consistency.

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
File ui/file_manager/audio_player/elements/repeat_button.html (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/repeat_button.html:12:
<files-toggle-ripple activated="{{isActive(repeatMode)}}"
class$="{{repeatMode}}"></files-toggle-ripple>
[[isActive(repeatMode)]] should be sufficient.

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
File ui/file_manager/audio_player/js/audio_player.js (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/js/audio_player.js:64: if(key === 'repeat' &&
typeof results[storageKey] === 'boolean') {
Let's make new key 'repeat-mode' for the string value and eliminate 'repeat' for
simplicity.

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/file_manage...
File ui/file_manager/file_manager_resources.grd (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/file_manage...
ui/file_manager/file_manager_resources.grd:159: <include
name="IDR_AUDIO_PLAYER_ELEMENTS_REPEAT_BUTTON_CSS"
file="audio_player/elements/repeat_button.css" flattenhtml="true" type="BINDATA"
/>
_CSS should be the first in alphabetical order.

[harukam, 2016-09-05 04:35:03]

Thanks for your review.
I have two questions about "audio_player.js".

Could you take a look?

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
File ui/file_manager/audio_player/elements/audio_player.js (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/audio_player.js:274: advance_:
function(forward, repeatMode) {
On 2016/09/02 12:11:48, fukino wrote:
> I think we don't need to change this advance_ function.
> Actually, in repeat-one mode, we don't advance to the next song.
> 
> Maybe we can handle the repeat-one case in scheduleAutoAdvance_.

scheduleAutoAdvance_ is called only when error happens.
So, I think we have to change not only scheduleAutoAdvance_, but also
onAudioEnded if we do not change advance_ function.
Do you think I should change these two parts, or create a new function like
repeat_or_advance_(repeatMode)?

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/audio_player.js:283:
this.$.trackList.fire('current-track-index-changed');
On 2016/09/02 12:11:48, fukino wrote:
> On 2016/09/02 11:27:27, harukam wrote:
> > Question, do you think this line is necessary?
> > According to the definition of 'currentTrackIndexChanged' in
> > 'audio_player/elements/track_list.js' (observer of currentTrackIndex), it
> > doesn't seem do anything when the index doesn't change.
> 
> According to https://codereview.chromium.org/1681843002,
> the current-track-index-changed event is fired to restart playback when there
is
> only one song.
> 
> Please check if it works without the event firing when the mode is repeat-one
> and there is only one song and the user press next/prev button.

Thanks for the link. It works without the event firing in all three modes when
there is only one song.

I don't understand why we need to fire the event though currentTrackIndexChanged
function doesn't do anything when both new value and old one are same.
In addition, currentTrackIndexChanged isn't called when we do
"this.$.trackList.fire('current-track-index-changed')".

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
File ui/file_manager/audio_player/elements/compiled_resources2.gyp (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/compiled_resources2.gyp:23: #     
'target_name': 'repeat_button',
On 2016/09/02 12:11:48, fukino wrote:
> Please sort targets based on their target names.

Done.

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
File ui/file_manager/audio_player/elements/control_panel.js (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/control_panel.js:83: repeat: {
On 2016/09/02 12:11:48, fukino wrote:
> This property should be repeatMode for consistency.

Agree. Thanks.

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
File ui/file_manager/audio_player/elements/repeat_button.html (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/repeat_button.html:12:
<files-toggle-ripple activated="{{isActive(repeatMode)}}"
class$="{{repeatMode}}"></files-toggle-ripple>
On 2016/09/02 12:11:48, fukino wrote:
> [[isActive(repeatMode)]] should be sufficient.

Acknowledged.

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
File ui/file_manager/audio_player/js/audio_player.js (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/js/audio_player.js:64: if(key === 'repeat' &&
typeof results[storageKey] === 'boolean') {
On 2016/09/02 12:11:48, fukino wrote:
> Let's make new key 'repeat-mode' for the string value and eliminate 'repeat'
for
> simplicity.

Acknowledged.

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/file_manage...
File ui/file_manager/file_manager_resources.grd (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/file_manage...
ui/file_manager/file_manager_resources.grd:159: <include
name="IDR_AUDIO_PLAYER_ELEMENTS_REPEAT_BUTTON_CSS"
file="audio_player/elements/repeat_button.css" flattenhtml="true" type="BINDATA"
/>
On 2016/09/02 12:11:49, fukino wrote:
> _CSS should be the first in alphabetical order.

Sorry for the carelessness. Done.

[fukino, 2016-09-05 11:44:13]

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
File ui/file_manager/audio_player/elements/audio_player.js (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/audio_player.js:274: advance_:
function(forward, repeatMode) {
On 2016/09/05 04:35:03, harukam wrote:
> On 2016/09/02 12:11:48, fukino wrote:
> > I think we don't need to change this advance_ function.
> > Actually, in repeat-one mode, we don't advance to the next song.
> > 
> > Maybe we can handle the repeat-one case in scheduleAutoAdvance_.
> 
> scheduleAutoAdvance_ is called only when error happens.
> So, I think we have to change not only scheduleAutoAdvance_, but also
> onAudioEnded if we do not change advance_ function.
> Do you think I should change these two parts, or create a new function like
> repeat_or_advance_(repeatMode)?

I think this should work:
1) On onAudioEnded, we repeat the current song if repeat mode is repeat-one.
2) On onAudioError, we should stop playing, instead of schedule auto advance, if
repeat mode is repeat-one.

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/audio_player.js:283:
this.$.trackList.fire('current-track-index-changed');
On 2016/09/05 04:35:03, harukam wrote:
> On 2016/09/02 12:11:48, fukino wrote:
> > On 2016/09/02 11:27:27, harukam wrote:
> > > Question, do you think this line is necessary?
> > > According to the definition of 'currentTrackIndexChanged' in
> > > 'audio_player/elements/track_list.js' (observer of currentTrackIndex), it
> > > doesn't seem do anything when the index doesn't change.
> > 
> > According to https://codereview.chromium.org/1681843002,
> > the current-track-index-changed event is fired to restart playback when
there
> is
> > only one song.
> > 
> > Please check if it works without the event firing when the mode is
repeat-one
> > and there is only one song and the user press next/prev button.
> 
> Thanks for the link. It works without the event firing in all three modes when
> there is only one song.
> 
> I don't understand why we need to fire the event though
currentTrackIndexChanged
> function doesn't do anything when both new value and old one are same.
> In addition, currentTrackIndexChanged isn't called when we do
> "this.$.trackList.fire('current-track-index-changed')".

Maybe we can drop the current-track-index-changed from advance_, but let's keep
it as it is until we can understand why it is added in the previous CL.

[harukam, 2016-09-09 04:54:51]

Sorry for being late.
Please take another look!

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
File ui/file_manager/audio_player/elements/audio_player.js (right):

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/audio_player.js:274: advance_:
function(forward, repeatMode) {
On 2016/09/05 11:44:13, fukino wrote:
> On 2016/09/05 04:35:03, harukam wrote:
> > On 2016/09/02 12:11:48, fukino wrote:
> > > I think we don't need to change this advance_ function.
> > > Actually, in repeat-one mode, we don't advance to the next song.
> > > 
> > > Maybe we can handle the repeat-one case in scheduleAutoAdvance_.
> > 
> > scheduleAutoAdvance_ is called only when error happens.
> > So, I think we have to change not only scheduleAutoAdvance_, but also
> > onAudioEnded if we do not change advance_ function.
> > Do you think I should change these two parts, or create a new function like
> > repeat_or_advance_(repeatMode)?
> 
> I think this should work:
> 1) On onAudioEnded, we repeat the current song if repeat mode is repeat-one.
> 2) On onAudioError, we should stop playing, instead of schedule auto advance,
if
> repeat mode is repeat-one.

OK. Thanks.

https://codereview.chromium.org/2305623003/diff/1/ui/file_manager/audio_playe...
ui/file_manager/audio_player/elements/audio_player.js:283:
this.$.trackList.fire('current-track-index-changed');
On 2016/09/05 11:44:13, fukino wrote:
> On 2016/09/05 04:35:03, harukam wrote:
> > On 2016/09/02 12:11:48, fukino wrote:
> > > On 2016/09/02 11:27:27, harukam wrote:
> > > > Question, do you think this line is necessary?
> > > > According to the definition of 'currentTrackIndexChanged' in
> > > > 'audio_player/elements/track_list.js' (observer of currentTrackIndex),
it
> > > > doesn't seem do anything when the index doesn't change.
> > > 
> > > According to https://codereview.chromium.org/1681843002,
> > > the current-track-index-changed event is fired to restart playback when
> there
> > is
> > > only one song.
> > > 
> > > Please check if it works without the event firing when the mode is
> repeat-one
> > > and there is only one song and the user press next/prev button.
> > 
> > Thanks for the link. It works without the event firing in all three modes
when
> > there is only one song.
> > 
> > I don't understand why we need to fire the event though
> currentTrackIndexChanged
> > function doesn't do anything when both new value and old one are same.
> > In addition, currentTrackIndexChanged isn't called when we do
> > "this.$.trackList.fire('current-track-index-changed')".
> 
> Maybe we can drop the current-track-index-changed from advance_, but let's
keep
> it as it is until we can understand why it is added in the previous CL.

Acknowledged.

[fukino, 2016-09-09 05:31:46]

It's getting close.

Please write CL description to explain what you did in this CL. (Created a
repeat-button to have 2 states as the ON in toggle-button, etc...)

Also, TEST= should be none for non-trivial CLs. At least, please write what you
manually tested.

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
File ui/file_manager/audio_player/elements/control_panel.css (right):

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/control_panel.css:141:
.media-button.repeatMode {
Use .repeat-mode to be consistent with .shuffle-mode etc.

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
File ui/file_manager/audio_player/elements/control_panel.html (right):

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/control_panel.html:27: id="repeatMode"
I think we can still use id #repeat for this button. (e.g. the toggle button for
change shuffle mode has simply #shuffle.)

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/control_panel.html:28: class="repeatMode
media-button"
class="repeat-mode ..

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
File ui/file_manager/audio_player/elements/control_panel.js (right):

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/control_panel.js:297:
this.$.repeatMode.setAttribute('aria-label', ariaLabels.repeatMode);
ariaLabels.repeatMode is undefined.

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
File ui/file_manager/audio_player/elements/repeat_button.css (right):

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/repeat_button.css:38:
files-toggle-ripple::shadow .ripple {
Please remove this line.
It was already removed in the latest files_icon_button.html

[harukam, 2016-09-09 11:49:47]

Description was changed from

==========
Support to repeat one song in Audio Player.

BUG=524884
TEST=none
==========

to

==========
Support to repeat one song in Audio Player.

BUG=524884
TEST=manually tested

Repeat mode can be 3 states.
A) Stop playing when all songs have been played.
B) Keep on playing by same order when all songs have been played.
C) Set a single song to repeat.

Created a repeat-button that has 1 state (A) as the OFF, and 2 states (B/C) as
the ON in toggle-button in order to show all 3 states visually.
==========

[harukam, 2016-09-09 11:53:45]

Thank you for reviewing.
Updated issue description.

Please take a look.

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
File ui/file_manager/audio_player/elements/control_panel.css (right):

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/control_panel.css:141:
.media-button.repeatMode {
On 2016/09/09 05:31:45, fukino wrote:
> Use .repeat-mode to be consistent with .shuffle-mode etc.

Acknowledged.

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
File ui/file_manager/audio_player/elements/control_panel.html (right):

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/control_panel.html:27: id="repeatMode"
On 2016/09/09 05:31:45, fukino wrote:
> I think we can still use id #repeat for this button. (e.g. the toggle button
for
> change shuffle mode has simply #shuffle.)

Acknowledged.

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/control_panel.html:28: class="repeatMode
media-button"
On 2016/09/09 05:31:45, fukino wrote:
> class="repeat-mode ..

Acknowledged.

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
File ui/file_manager/audio_player/elements/control_panel.js (right):

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/control_panel.js:297:
this.$.repeatMode.setAttribute('aria-label', ariaLabels.repeatMode);
On 2016/09/09 05:31:45, fukino wrote:
> ariaLabels.repeatMode is undefined.

Acknowledged.

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
File ui/file_manager/audio_player/elements/repeat_button.css (right):

https://codereview.chromium.org/2305623003/diff/40001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/repeat_button.css:38:
files-toggle-ripple::shadow .ripple {
On 2016/09/09 05:31:45, fukino wrote:
> Please remove this line.
> It was already removed in the latest files_icon_button.html

Thanks.

[fukino, 2016-09-09 11:59:39]

LGTM with a nit.

Please write the CL description between the title and BUG= linke.

Thanks!

[harukam, 2016-09-12 02:14:03]

Description was changed from

==========
Support to repeat one song in Audio Player.

BUG=524884
TEST=manually tested

Repeat mode can be 3 states.
A) Stop playing when all songs have been played.
B) Keep on playing by same order when all songs have been played.
C) Set a single song to repeat.

Created a repeat-button that has 1 state (A) as the OFF, and 2 states (B/C) as
the ON in toggle-button in order to show all 3 states visually.
==========

to

==========
Support to repeat one song in Audio Player.

Repeat mode can be 3 states.
A) Stop playing when all songs have been played.
B) Keep on playing by same order when all songs have been played.
C) Set a single song to repeat.

Created a repeat-button that has 1 state (A) as the OFF, and 2 states (B/C) as
the ON in toggle-button in order to show all 3 states visually.

BUG=524884
TEST=manually tested
==========

[harukam, 2016-09-12 02:14:11]

The CQ bit was checked by harukam@google.com

[commit-bot: I haz the power, 2016-09-12 02:14:22]

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, 2016-09-12 03:24:00]

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

[commit-bot: I haz the power, 2016-09-12 03:24:01]

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

[harukam, 2016-09-13 07:32:47]

fukino-san, I added some tests for new repeat mode.
Please take a look.

[fukino, 2016-09-13 11:59:49]

https://codereview.chromium.org/2305623003/diff/80001/ui/file_manager/integra...
File ui/file_manager/integration_tests/file_manager/open_audio_files.js (right):

https://codereview.chromium.org/2305623003/diff/80001/ui/file_manager/integra...
ui/file_manager/integration_tests/file_manager/open_audio_files.js:399:
this.next);
this.next is called twice in this function.
You need to add one more function for the second call of this
audioPlayerApp.callRemoteTestUtil.

https://codereview.chromium.org/2305623003/diff/80001/ui/file_manager/integra...
ui/file_manager/integration_tests/file_manager/open_audio_files.js:636:
audioPlayerApp.callRemoteTestUtil(
ditto

https://codereview.chromium.org/2305623003/diff/80001/ui/file_manager/integra...
ui/file_manager/integration_tests/file_manager/open_audio_files.js:649:
audioPlayerApp.waitForElement(audioAppId, query).then(this.next);
I think this test doesn't wait the end of current track, so it doesn't confirm
that the track is repeated.
Maybe we need to check playcount?

[harukam, 2016-09-14 05:58:09]

Thank you for your review.

Please take a look again!

https://codereview.chromium.org/2305623003/diff/80001/ui/file_manager/integra...
File ui/file_manager/integration_tests/file_manager/open_audio_files.js (right):

https://codereview.chromium.org/2305623003/diff/80001/ui/file_manager/integra...
ui/file_manager/integration_tests/file_manager/open_audio_files.js:399:
this.next);
On 2016/09/13 11:59:49, fukino wrote:
> this.next is called twice in this function.
> You need to add one more function for the second call of this
> audioPlayerApp.callRemoteTestUtil.

Acknowledged.

https://codereview.chromium.org/2305623003/diff/80001/ui/file_manager/integra...
ui/file_manager/integration_tests/file_manager/open_audio_files.js:636:
audioPlayerApp.callRemoteTestUtil(
On 2016/09/13 11:59:49, fukino wrote:
> ditto

Acknowledged.

https://codereview.chromium.org/2305623003/diff/80001/ui/file_manager/integra...
ui/file_manager/integration_tests/file_manager/open_audio_files.js:649:
audioPlayerApp.waitForElement(audioAppId, query).then(this.next);
On 2016/09/13 11:59:49, fukino wrote:
> I think this test doesn't wait the end of current track, so it doesn't confirm
> that the track is repeated.
> Maybe we need to check playcount?

Thanks.

[fukino, 2016-09-14 11:05:54]

lgtm with nits.

https://codereview.chromium.org/2305623003/diff/100001/ui/file_manager/integr...
File ui/file_manager/integration_tests/file_manager/open_audio_files.js (right):

https://codereview.chromium.org/2305623003/diff/100001/ui/file_manager/integr...
ui/file_manager/integration_tests/file_manager/open_audio_files.js:396:
function(element) {
nit: Please remove |element|, as it is not used in this function.

https://codereview.chromium.org/2305623003/diff/100001/ui/file_manager/integr...
ui/file_manager/integration_tests/file_manager/open_audio_files.js:639:
function(element) {
ditto.

[harukam, 2016-09-15 01:33:03]

Thanks!

https://codereview.chromium.org/2305623003/diff/100001/ui/file_manager/integr...
File ui/file_manager/integration_tests/file_manager/open_audio_files.js (right):

https://codereview.chromium.org/2305623003/diff/100001/ui/file_manager/integr...
ui/file_manager/integration_tests/file_manager/open_audio_files.js:396:
function(element) {
On 2016/09/14 11:05:54, fukino wrote:
> nit: Please remove |element|, as it is not used in this function.

Done.

https://codereview.chromium.org/2305623003/diff/100001/ui/file_manager/integr...
ui/file_manager/integration_tests/file_manager/open_audio_files.js:639:
function(element) {
On 2016/09/14 11:05:54, fukino wrote:
> ditto.

Done.

[harukam, 2016-09-15 01:33:13]

The CQ bit was checked by harukam@google.com

[harukam, 2016-09-15 01:33:14]

The patchset sent to the CQ was uploaded after l-g-t-m from fukino@chromium.org
Link to the patchset: https://codereview.chromium.org/2305623003/#ps120001
(title: "Add a test for 3 repeat mode.")

[commit-bot: I haz the power, 2016-09-15 01:33:44]

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, 2016-09-15 02:07:11]

Description was changed from

==========
Support to repeat one song in Audio Player.

Repeat mode can be 3 states.
A) Stop playing when all songs have been played.
B) Keep on playing by same order when all songs have been played.
C) Set a single song to repeat.

Created a repeat-button that has 1 state (A) as the OFF, and 2 states (B/C) as
the ON in toggle-button in order to show all 3 states visually.

BUG=524884
TEST=manually tested
==========

to

==========
Support to repeat one song in Audio Player.

Repeat mode can be 3 states.
A) Stop playing when all songs have been played.
B) Keep on playing by same order when all songs have been played.
C) Set a single song to repeat.

Created a repeat-button that has 1 state (A) as the OFF, and 2 states (B/C) as
the ON in toggle-button in order to show all 3 states visually.

BUG=524884
TEST=manually tested
==========

[commit-bot: I haz the power, 2016-09-15 02:07:13]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-09-15 02:10:15]

Description was changed from

==========
Support to repeat one song in Audio Player.

Repeat mode can be 3 states.
A) Stop playing when all songs have been played.
B) Keep on playing by same order when all songs have been played.
C) Set a single song to repeat.

Created a repeat-button that has 1 state (A) as the OFF, and 2 states (B/C) as
the ON in toggle-button in order to show all 3 states visually.

BUG=524884
TEST=manually tested
==========

to

==========
Support to repeat one song in Audio Player.

Repeat mode can be 3 states.
A) Stop playing when all songs have been played.
B) Keep on playing by same order when all songs have been played.
C) Set a single song to repeat.

Created a repeat-button that has 1 state (A) as the OFF, and 2 states (B/C) as
the ON in toggle-button in order to show all 3 states visually.

BUG=524884
TEST=manually tested

Committed: https://crrev.com/ce5ebc11016280f6ef7b6bf69a06b9f5716caa75
Cr-Commit-Position: refs/heads/master@{#418755}
==========

[commit-bot: I haz the power, 2016-09-15 02:10:16]

Patchset 7 (id:??) landed as
https://crrev.com/ce5ebc11016280f6ef7b6bf69a06b9f5716caa75
Cr-Commit-Position: refs/heads/master@{#418755}

[harukam, 2016-09-15 09:31:30]

Description was changed from

==========
Support to repeat one song in Audio Player.

Repeat mode can be 3 states.
A) Stop playing when all songs have been played.
B) Keep on playing by same order when all songs have been played.
C) Set a single song to repeat.

Created a repeat-button that has 1 state (A) as the OFF, and 2 states (B/C) as
the ON in toggle-button in order to show all 3 states visually.

BUG=524884
TEST=manually tested

Committed: https://crrev.com/ce5ebc11016280f6ef7b6bf69a06b9f5716caa75
Cr-Commit-Position: refs/heads/master@{#418755}
==========

to

==========
Support to repeat one song in Audio Player.

Repeat mode can be 3 states.
A) Stop playing when all songs have been played.
B) Keep on playing by same order when all songs have been played.
C) Set a single song to repeat.

Created a repeat-button that has 1 state (A) as the OFF, and 2 states (B/C) as
the ON in toggle-button in order to show all 3 states visually.

BUG=524884
TEST=browser_tests --gtest_filter=OpenAudioFiles*

Committed: https://crrev.com/ce5ebc11016280f6ef7b6bf69a06b9f5716caa75
Cr-Commit-Position: refs/heads/master@{#418755}
==========

[ulaygyi05pt, 2016-10-05 02:38:52]

On 2016/09/15 02:10:16, commit-bot: I haz the power wrote:
> Patchset 7 (id:??) landed as
> https://crrev.com/ce5ebc11016280f6ef7b6bf69a06b9f5716caa75
> Cr-Commit-Position: refs/heads/master@{#418755}