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

Issue 338243005: Revert of Revert of Support casting for embedded YT videos (Closed)

Created:
6 years, 6 months ago by May
Modified:
6 years, 6 months ago
CC:
chromium-reviews, favayvod+watch_chromium.org, jam, mcasas+watch_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, wjia+watch_chromium.org, mark a. foltz
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Revert of Revert of Support casting for embedded YT videos (https://codereview.chromium.org/339613002/) Reason for revert: Re-landing casting embedded YT feature. Original issue's description: > Revert of Support casting for embedded YT videos (https://codereview.chromium.org/302453012/) > > Reason for revert: > It appears to break compile on android: > > ../../media/base/android/media_source_player_unittest.cc: In constructor 'media::MediaSourcePlayerTest::MediaSourcePlayerTest()': > ../../media/base/android/media_source_player_unittest.cc:172:40:error: no matching function for call to 'media::MediaSourcePlayer::MediaSourcePlayer(int, media::MockMediaPlayerManager*, base::Callback<void(int)>, base::Callback<void(int)>, scoped_ptr<media::DemuxerAndroid>)' > surface_texture_a_is_next_(true) {} > ^ > ../../media/base/android/media_source_player_unittest.cc:172:40: note: candidates are: > In file included from ../../base/basictypes.h:18:0, > from ../../media/base/android/media_source_player_unittest.cc:7: > ../../media/base/android/media_source_player.h:269:28: note: media::MediaSourcePlayer::MediaSourcePlayer(const media::MediaSourcePlayer&) > DISALLOW_COPY_AND_ASSIGN(MediaSourcePlayer); > ^ > ../../base/macros.h:29:3: note: in definition of macro 'DISALLOW_COPY_AND_ASSIGN' > TypeName(const TypeName&); \ > ^ > ../../media/base/android/media_source_player.h:269:28: note: candidate expects 1 argument, 5 provided > DISALLOW_COPY_AND_ASSIGN(MediaSourcePlayer); > ^ > ../../base/macros.h:29:3: note: in definition of macro 'DISALLOW_COPY_AND_ASSIGN' > TypeName(const TypeName&); \ > ^ > In file included from ../../media/base/android/media_source_player_unittest.cc:15:0: > ../../media/base/android/media_source_player.h:41:3: note: media::MediaSourcePlayer::MediaSourcePlayer(int, media::MediaPlayerManager*, const RequestMediaResourcesCB&, const ReleaseMediaResourcesCB&, scoped_ptr<media::DemuxerAndroid>, const GURL&) > MediaSourcePlayer(int player_id, > ^ > ../../media/base/android/media_source_player.h:41:3: note: candidate expects 6 arguments, 5 provided > ninja: build stopped: subcommand failed. > > > Original issue's description: > > Support casting for embedded YT videos > > > > This is part 1 of the code to start supporting embedded YT for cast. The main > > issue is that YT embedded videos need to be identified via the frame that > > contains it. This CL sets the frame URL of each video in the MediaPlayerAndroid > > object. > > > > NOTRY=true > > > > Clank downstream piece to come later. > > Min / Aaron: will send you the doc I have that explains this in more detail. > > > > BUG=358573 > > > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=277425 > > NOTRY=true > NOTREECHECK=true > TBR=maybelle@chromium.org TBR=qinmin@chromium.org,acolwell@chromium.org,avayvod@chromium.org,aberent@chromium.org,mfoltz@chromium.org,miguelg@chromium.org,cpalmer@chromium.org,binjin@chromium.org

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+83 lines, -66 lines) Patch
M content/browser/media/android/browser_media_player_manager.h View 3 chunks +4 lines, -10 lines 0 comments Download
M content/browser/media/android/browser_media_player_manager.cc View 4 chunks +22 lines, -24 lines 0 comments Download
M content/common/media/media_player_messages_android.h View 2 chunks +15 lines, -15 lines 0 comments Download
M content/public/test/android/javatests/src/org/chromium/content/browser/test/util/DOMUtils.java View 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/media/android/renderer_media_player_manager.h View 2 chunks +4 lines, -2 lines 0 comments Download
M content/renderer/media/android/renderer_media_player_manager.cc View 1 chunk +12 lines, -4 lines 0 comments Download
M content/renderer/media/android/webmediaplayer_android.cc View 1 chunk +2 lines, -1 line 0 comments Download
M media/base/android/media_player_android.h View 2 chunks +7 lines, -1 line 0 comments Download
M media/base/android/media_player_android.cc View 1 chunk +4 lines, -2 lines 0 comments Download
M media/base/android/media_player_bridge.h View 1 chunk +2 lines, -1 line 0 comments Download
M media/base/android/media_player_bridge.cc View 1 chunk +4 lines, -2 lines 0 comments Download
M media/base/android/media_source_player.h View 1 chunk +2 lines, -1 line 0 comments Download
M media/base/android/media_source_player.cc View 1 chunk +4 lines, -2 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
May
Created Revert of Revert of Support casting for embedded YT videos
6 years, 6 months ago (2014-06-17 10:50:50 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/maybelle@chromium.org/338243005/1
6 years, 6 months ago (2014-06-17 10:52:25 UTC) #2
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-06-17 10:52:26 UTC) #3
commit-bot: I haz the power
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an ...
6 years, 6 months ago (2014-06-17 10:52:27 UTC) #4
aberent
The CQ bit was unchecked by aberent@chromium.org
6 years, 6 months ago (2014-06-17 10:52:29 UTC) #5
binjin
The CQ bit was unchecked by binjin@chromium.org
6 years, 6 months ago (2014-06-17 10:53:47 UTC) #6
binjin
On 2014/06/17 10:50:50, may wrote: > Created Revert of Revert of Support casting for embedded ...
6 years, 6 months ago (2014-06-17 10:54:31 UTC) #7
whywhat
lgtm
6 years, 6 months ago (2014-06-17 10:55:12 UTC) #8
qinmin
6 years, 6 months ago (2014-06-17 13:41:28 UTC) #9
lgtm.

It's better to create PS#1 from the original CL, and then have your new changes
as PS#2

Powered by Google App Engine
This is Rietveld 408576698