Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(28)

Issue 2898553002: [Media,Remoting] Remove the disable remoting button on Android (Closed)

Created:
1 year, 1 month ago by whywhat
Modified:
1 year ago
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, 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, rwlbuis, nessy, Srirama
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Media,Remoting] Remove the disable remoting button on Android The --new-remote-playback-pipeline flag will stay disabled on desktop by default. We want to reuse the media remoting interstitial with the new remote playback pipeline instead of the old fake video frame with text generated using GL. The interstitial currently has a button on it that says "Play on both devices" that turns remoting off. This CL allows the button to be removed. BUG=517102 TEST=initiate remoting with --new-remote-playback-pipeline and check the button is not there Review-Url: https://codereview.chromium.org/2898553002 Cr-Commit-Position: refs/heads/master@{#475363} Committed: https://chromium.googlesource.com/chromium/src/+/cd64baef7c36f9763e0d152ca3a3723077b80a7f

Patch Set 1 #

Patch Set 2 : Don't show the exit button on Android only for now #

Patch Set 3 : Use Android CSS to disable the button #

Total comments: 4

Patch Set 4 : Removed the style check for event listeners #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -0 lines) Patch
M third_party/WebKit/Source/core/css/mediaControlsAndroid.css View 1 2 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 41 (18 generated)
whywhat
PTaL
1 year, 1 month ago (2017-05-19 22:44:24 UTC) #2
xjz
Can you please hold off this change? We are not quite ready yet. We need ...
1 year, 1 month ago (2017-05-22 20:49:41 UTC) #7
whywhat
On 2017/05/22 at 20:49:41, xjz wrote: > Can you please hold off this change? We ...
1 year, 1 month ago (2017-05-22 21:35:57 UTC) #8
xjz
On 2017/05/22 21:35:57, whywhat wrote: > On 2017/05/22 at 20:49:41, xjz wrote: > > Can ...
1 year, 1 month ago (2017-05-22 21:58:47 UTC) #9
whywhat
Don't show the exit button on Android only for now
1 year ago (2017-05-24 01:25:21 UTC) #10
whywhat
PTAL I didn't want to introduce a separate runtime feature flag and it seems that ...
1 year ago (2017-05-24 01:29:15 UTC) #12
mlamouri (slow - plz ping)
avayvod@, I am not sure what this is trying to do: can you explain in ...
1 year ago (2017-05-24 16:42:05 UTC) #17
whywhat
On 2017/05/24 at 16:42:05, mlamouri wrote: > avayvod@, I am not sure what this is ...
1 year ago (2017-05-24 20:40:20 UTC) #19
xjz
lgtm
1 year ago (2017-05-24 21:14:36 UTC) #20
whywhat
Use Android CSS to disable the button
1 year ago (2017-05-24 23:50:42 UTC) #21
whywhat
rune@, PTaL at mediacontrolsandroid.css, thanks!
1 year ago (2017-05-25 02:35:31 UTC) #23
xjz
https://codereview.chromium.org/2898553002/diff/40001/third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp File third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp (right): https://codereview.chromium.org/2898553002/diff/40001/third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp#newcode68 third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp:68: if (style->Display() == EDisplay::kNone) Have you tested on this? ...
1 year ago (2017-05-25 19:02:18 UTC) #24
whywhat
https://codereview.chromium.org/2898553002/diff/40001/third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp File third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp (right): https://codereview.chromium.org/2898553002/diff/40001/third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp#newcode68 third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp:68: if (style->Display() == EDisplay::kNone) On 2017/05/25 at 19:02:18, xjz ...
1 year ago (2017-05-25 20:26:30 UTC) #25
xjz
https://codereview.chromium.org/2898553002/diff/40001/third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp File third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp (right): https://codereview.chromium.org/2898553002/diff/40001/third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp#newcode68 third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp:68: if (style->Display() == EDisplay::kNone) On 2017/05/25 20:26:29, whywhat wrote: ...
1 year ago (2017-05-25 21:20:34 UTC) #26
whywhat
-rune +chrishtr for mediacontrolsandroid.css PTaL
1 year ago (2017-05-26 01:40:19 UTC) #28
rune
https://codereview.chromium.org/2898553002/diff/40001/third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp File third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp (right): https://codereview.chromium.org/2898553002/diff/40001/third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp#newcode68 third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp:68: if (style->Display() == EDisplay::kNone) On 2017/05/25 19:02:18, xjz wrote: ...
1 year ago (2017-05-26 11:17:58 UTC) #30
mlamouri (slow - plz ping)
I wouldn't mind having the events there all the time as it would be a ...
1 year ago (2017-05-26 13:34:00 UTC) #31
whywhat
Removed the style check for event listeners
1 year ago (2017-05-26 16:54:26 UTC) #32
whywhat
I'm all for simplification. rune@ PTAL, thanks!
1 year ago (2017-05-26 16:55:16 UTC) #33
xjz
Having the event listeners lgtm too. :)
1 year ago (2017-05-26 16:59:45 UTC) #34
rune
rs lgtm
1 year ago (2017-05-29 07:52:01 UTC) #35
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/2898553002/60001
1 year ago (2017-05-29 16:27:40 UTC) #38
commit-bot: I haz the power
1 year ago (2017-05-29 17:42:52 UTC) #41
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/cd64baef7c36f9763e0d152ca3a3...

Powered by Google App Engine
This is Rietveld 408576698