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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months ago by whywhat
Modified:
4 months, 3 weeks 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 #

Messages

Total messages: 41 (18 generated)
whywhat
PTaL
5 months 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 ...
5 months 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 ...
5 months 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 ...
5 months ago (2017-05-22 21:58:47 UTC) #9
whywhat
Don't show the exit button on Android only for now
5 months 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 ...
5 months 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 ...
5 months 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 ...
5 months ago (2017-05-24 20:40:20 UTC) #19
xjz
lgtm
5 months ago (2017-05-24 21:14:36 UTC) #20
whywhat
Use Android CSS to disable the button
5 months ago (2017-05-24 23:50:42 UTC) #21
whywhat
rune@, PTaL at mediacontrolsandroid.css, thanks!
5 months 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? ...
5 months 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 ...
5 months 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: ...
5 months ago (2017-05-25 21:20:34 UTC) #26
whywhat
-rune +chrishtr for mediacontrolsandroid.css PTaL
5 months 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: ...
4 months, 4 weeks 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 ...
4 months, 4 weeks ago (2017-05-26 13:34:00 UTC) #31
whywhat
Removed the style check for event listeners
4 months, 4 weeks ago (2017-05-26 16:54:26 UTC) #32
whywhat
I'm all for simplification. rune@ PTAL, thanks!
4 months, 4 weeks ago (2017-05-26 16:55:16 UTC) #33
xjz
Having the event listeners lgtm too. :)
4 months, 4 weeks ago (2017-05-26 16:59:45 UTC) #34
rune
rs lgtm
4 months, 3 weeks 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
4 months, 3 weeks ago (2017-05-29 16:27:40 UTC) #38
commit-bot: I haz the power
4 months, 3 weeks 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa