Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(215)

Issue 2563853003: [MediaControls] Listen to durationchange instead of letting HTML call methods of MediaControls (Closed)

Created:
4 years ago by Zhiqiang Zhang (Slow)
Modified:
4 years ago
CC:
blink-reviews, blink-reviews-html_chromium.org, chromium-reviews, dglazkov+blink, eric.carlson_apple.com, feature-media-reviews_chromium.org, fs, gasubic, mlamouri+watch-blink_chromium.org, nessy, Srirama, vcarbune.chromium
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[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}

Patch Set 1 #

Total comments: 2

Patch Set 2 : doing the same as timeupdate #

Total comments: 2

Patch Set 3 : addressed nits from mlamouri@ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -2 lines) Patch
M third_party/WebKit/Source/core/html/HTMLMediaElement.cpp View 1 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/html/shadow/MediaControls.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/html/shadow/MediaControls.cpp View 1 2 1 chunk +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp View 1 2 chunks +6 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (12 generated)
Zhiqiang Zhang (Slow)
4 years ago (2016-12-12 16:31:33 UTC) #2
mlamouri (slow - plz ping)
https://codereview.chromium.org/2563853003/diff/1/third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp File third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp (right): https://codereview.chromium.org/2563853003/diff/1/third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp#newcode66 third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp:66: m_mediaControls->reset(); What about adding onDurationChange to MediaControls? Can we ...
4 years ago (2016-12-13 13:13:26 UTC) #3
Zhiqiang Zhang (Slow)
PTAL. Now I'm doing exactly what we do for `timeupdate` event https://codereview.chromium.org/2563853003/diff/1/third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp File third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp (right): ...
4 years ago (2016-12-13 15:49:35 UTC) #4
mlamouri (slow - plz ping)
lgtm with comments applied. If the tests are passing :) https://codereview.chromium.org/2563853003/diff/20001/third_party/WebKit/Source/core/html/shadow/MediaControls.cpp File third_party/WebKit/Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/2563853003/diff/20001/third_party/WebKit/Source/core/html/shadow/MediaControls.cpp#newcode720 ...
4 years ago (2016-12-13 20:00:00 UTC) #7
Zhiqiang Zhang (Slow)
https://codereview.chromium.org/2563853003/diff/20001/third_party/WebKit/Source/core/html/shadow/MediaControls.cpp File third_party/WebKit/Source/core/html/shadow/MediaControls.cpp (right): https://codereview.chromium.org/2563853003/diff/20001/third_party/WebKit/Source/core/html/shadow/MediaControls.cpp#newcode720 third_party/WebKit/Source/core/html/shadow/MediaControls.cpp:720: makeTransparent(); On 2016/12/13 20:00:00, mlamouri wrote: > I don't ...
4 years ago (2016-12-13 20:13:21 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2563853003/40001
4 years ago (2016-12-13 21:43:21 UTC) #15
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years ago (2016-12-13 22:17:28 UTC) #18
commit-bot: I haz the power
4 years ago (2016-12-13 22:19:42 UTC) #20
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/bc2d34472dd3e60dabae58359a7cae0c49ae6555
Cr-Commit-Position: refs/heads/master@{#438309}

Powered by Google App Engine
This is Rietveld 408576698