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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 364033003: Eliminate MediaPlayer abstraction(network state) (Closed)

Created:
6 years, 5 months ago by Srirama
Modified:
6 years, 5 months ago
CC:
arv+blink, blink-reviews, blink-reviews-html_chromium.org, Rik, Inactive, danakj, dglazkov+blink, krit, eric.carlson_apple.com, feature-media-reviews_chromium.org, fs, gasubic, jamesr, jbroman, pdr., rwlbuis, Stephen Chennney, nessy, vcarbune.chromium, watchdog-blink-watchlist_google.com
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Eliminate MediaPlayer abstraction(network state) BUG=350571 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178010

Patch Set 1 #

Patch Set 2 : Modified the approach as suggested #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -36 lines) Patch
M Source/core/html/HTMLMediaElement.h View 1 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/html/HTMLMediaElement.cpp View 1 7 chunks +19 lines, -15 lines 2 comments Download
M Source/platform/graphics/media/MediaPlayer.h View 1 1 chunk +0 lines, -3 lines 0 comments Download
M Source/web/AssertMatchingEnums.cpp View 1 1 chunk +0 lines, -8 lines 0 comments Download
M Source/web/WebMediaPlayerClientImpl.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M Source/web/WebMediaPlayerClientImpl.cpp View 1 1 chunk +0 lines, -7 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
Srirama
Removed networkstate related things from mediaplayer and webmediaplayerclientimpl. I have added extra states in HTMLMediaElement ...
6 years, 5 months ago (2014-07-03 17:01:21 UTC) #1
philipj_slow
Not LGTM, you've made Web-facing changes with the networkState constants that don't match the spec: ...
6 years, 5 months ago (2014-07-04 21:41:54 UTC) #2
Srirama
On 2014/07/04 21:41:54, philipj wrote: > Not LGTM, you've made Web-facing changes with the networkState ...
6 years, 5 months ago (2014-07-05 03:39:46 UTC) #3
Srirama
Modified as per the suggestion by using webmediaplayer states instead of disturbing HTMLMediaElement. PTAL
6 years, 5 months ago (2014-07-09 14:30:43 UTC) #4
philipj_slow
LGTM % nit https://codereview.chromium.org/364033003/diff/60001/Source/core/html/HTMLMediaElement.cpp File Source/core/html/HTMLMediaElement.cpp (right): https://codereview.chromium.org/364033003/diff/60001/Source/core/html/HTMLMediaElement.cpp#newcode1629 Source/core/html/HTMLMediaElement.cpp:1629: setNetworkState(webMediaPlayer()->networkState()); You should also be able ...
6 years, 5 months ago (2014-07-12 20:50:55 UTC) #5
Srirama
abarth@: owners review for web/ and platform/ please. https://codereview.chromium.org/364033003/diff/60001/Source/core/html/HTMLMediaElement.cpp File Source/core/html/HTMLMediaElement.cpp (right): https://codereview.chromium.org/364033003/diff/60001/Source/core/html/HTMLMediaElement.cpp#newcode1629 Source/core/html/HTMLMediaElement.cpp:1629: setNetworkState(webMediaPlayer()->networkState()); ...
6 years, 5 months ago (2014-07-13 05:30:17 UTC) #6
abarth-chromium
LGTM
6 years, 5 months ago (2014-07-13 06:07:46 UTC) #7
Srirama
The CQ bit was checked by srirama.m@samsung.com
6 years, 5 months ago (2014-07-13 08:07:49 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/srirama.m@samsung.com/364033003/60001
6 years, 5 months ago (2014-07-13 08:08:28 UTC) #9
commit-bot: I haz the power
6 years, 5 months ago (2014-07-13 08:11:38 UTC) #10
Message was sent while issue was closed. 
Change committed as 178010

Powered by Google App Engine
This is Rietveld 408576698