[MediaControls] Listen to durationchange instead of letting HTML call methods of MediaControls

[MediaControls] Listen to durationchange instead of letting HTML call methods of MediaControls

BUG=662761
Committed: https://crrev.com/bc2d34472dd3e60dabae58359a7cae0c49ae6555
Cr-Commit-Position: refs/heads/master@{#438309}

[Zhiqiang Zhang (Slow), 2016-12-12 16:31:32]

zqzhang@chromium.org changed reviewers:
+ mlamouri@chromium.org

[mlamouri (slow - plz ping), 2016-12-13 13:13:26]

https://codereview.chromium.org/2563853003/diff/1/third_party/WebKit/Source/c...
File
third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp
(right):

https://codereview.chromium.org/2563853003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp:66:
m_mediaControls->reset();
What about adding onDurationChange to MediaControls? Can we just update the
duration related things when the duration has changed instead of resetting? I
would think this might work if you don't remove the first "reset()" which is
related to ready state change (ie. loadedmetadata).

[Zhiqiang Zhang (Slow), 2016-12-13 15:49:35]

PTAL. Now I'm doing exactly what we do for `timeupdate` event

https://codereview.chromium.org/2563853003/diff/1/third_party/WebKit/Source/c...
File
third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp
(right):

https://codereview.chromium.org/2563853003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp:66:
m_mediaControls->reset();
On 2016/12/13 13:13:26, mlamouri wrote:
> What about adding onDurationChange to MediaControls? Can we just update the
> duration related things when the duration has changed instead of resetting? I
> would think this might work if you don't remove the first "reset()" which is
> related to ready state change (ie. loadedmetadata).

Done.

[mlamouri (slow - plz ping), 2016-12-13 19:55:29]

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

[commit-bot: I haz the power, 2016-12-13 19:56:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mlamouri (slow - plz ping), 2016-12-13 20:00:00]

lgtm with comments applied. If the tests are passing :)

https://codereview.chromium.org/2563853003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/shadow/MediaControls.cpp (right):

https://codereview.chromium.org/2563853003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/shadow/MediaControls.cpp:720:
makeTransparent();
I don't think you need to do all the transparency checks. The media controls are
using timeupdate to auto-hide. Duration change shouldn't worry about that. Only
the two first lines are needed.

[Zhiqiang Zhang (Slow), 2016-12-13 20:13:21]

https://codereview.chromium.org/2563853003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/shadow/MediaControls.cpp (right):

https://codereview.chromium.org/2563853003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/shadow/MediaControls.cpp:720:
makeTransparent();
On 2016/12/13 20:00:00, mlamouri wrote:
> I don't think you need to do all the transparency checks. The media controls
are
> using timeupdate to auto-hide. Duration change shouldn't worry about that.
Only
> the two first lines are needed.

Done.

[Zhiqiang Zhang (Slow), 2016-12-13 20:14:43]

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

[commit-bot: I haz the power, 2016-12-13 20:15:28]

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, 2016-12-13 21:06:43]

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

[commit-bot: I haz the power, 2016-12-13 21:06:44]

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_...)

[Zhiqiang Zhang (Slow), 2016-12-13 21:42:51]

The CQ bit was checked by zqzhang@chromium.org

[Zhiqiang Zhang (Slow), 2016-12-13 21:42:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/2563853003/#ps40001
(title: "addressed nits from mlamouri@")

[commit-bot: I haz the power, 2016-12-13 21:43:21]

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-12-13 22:17:01]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481665371955510,
"parent_rev": "286584cb9cf8ad1d057e3e9bbec5f3d909acd495", "commit_rev":
"73200d5d5f111ae63bac9c1545dee2e577471f3d"}

[commit-bot: I haz the power, 2016-12-13 22:17:27]

Description was changed from

==========
[MediaControls] Listen to durationchange instead of letting HTML call methods of
MediaControls

BUG=662761
==========

to

==========
[MediaControls] Listen to durationchange instead of letting HTML call methods of
MediaControls

BUG=662761

Review-Url: https://codereview.chromium.org/2563853003
==========

[commit-bot: I haz the power, 2016-12-13 22:17:28]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-13 22:19:39]

Description was changed from

==========
[MediaControls] Listen to durationchange instead of letting HTML call methods of
MediaControls

BUG=662761

Review-Url: https://codereview.chromium.org/2563853003
==========

to

==========
[MediaControls] Listen to durationchange instead of letting HTML call methods of
MediaControls

BUG=662761

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

[commit-bot: I haz the power, 2016-12-13 22:19:42]

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