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

Issue 2874943002: [Chromecast] Add service for "displaying" cast web contents. (Closed)

Created:
1 year, 3 months ago by thoren
Modified:
1 year, 2 months ago
Reviewers:
slan, halliwell, *Simeon
CC:
chromium-reviews, alokp+watch_chromium.org, lcwu+watch_chromium.org, halliwell+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Chromecast] Add service for "displaying" cast web contents. Audio-only Android builds should not start an activity, so this change adds the option of using a service. Rather than directly starting an activity to display cast apps, CastContentWindowAndroid uses CastWebContentsComponent to start and interact with whichever kind of component is appropriate for the build. This is controlled with the build flag 'display_web_contents_in_service', which defaults to is_cast_audio_only. BUG=internal 36596922 Review-Url: https://codereview.chromium.org/2874943002 Cr-Commit-Position: refs/heads/master@{#473670} Committed: https://chromium.googlesource.com/chromium/src/+/9540c6d6f520ad49921f0dc4e6b925324c623dfc

Patch Set 1 #

Total comments: 22

Patch Set 2 : Add service for "displaying" cast web contents. #

Patch Set 3 : Add service for "displaying" cast web contents. #

Total comments: 6

Patch Set 4 : Add service for "displaying" cast web contents. #

Total comments: 9

Patch Set 5 : [Chromecast] Add service for "displaying" cast web contents. #

Patch Set 6 : [Chromecast] Add service for "displaying" cast web contents. #

Patch Set 7 : [Chromecast] Add service for "displaying" cast web contents. #

Messages

Total messages: 49 (32 generated)
thoren
@slan: Because OWNERS @sanfin: PTAL
1 year, 3 months ago (2017-05-15 16:52:37 UTC) #6
Simeon
https://codereview.chromium.org/2874943002/diff/1/chromecast/browser/android/BUILD.gn File chromecast/browser/android/BUILD.gn (right): https://codereview.chromium.org/2874943002/diff/1/chromecast/browser/android/BUILD.gn#newcode73 chromecast/browser/android/BUILD.gn:73: junit_binary("cast_shell_java_junit_tests") { Let's call this "cast_shell_junit_tests" for now, since ...
1 year, 3 months ago (2017-05-15 19:47:24 UTC) #11
thoren
https://codereview.chromium.org/2874943002/diff/1/chromecast/browser/android/BUILD.gn File chromecast/browser/android/BUILD.gn (right): https://codereview.chromium.org/2874943002/diff/1/chromecast/browser/android/BUILD.gn#newcode73 chromecast/browser/android/BUILD.gn:73: junit_binary("cast_shell_java_junit_tests") { On 2017/05/15 19:47:24, Simeon wrote: > Let's ...
1 year, 3 months ago (2017-05-15 22:25:24 UTC) #15
Simeon
lgtm % a couple nits https://codereview.chromium.org/2874943002/diff/40001/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsComponent.java File chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsComponent.java (right): https://codereview.chromium.org/2874943002/diff/40001/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsComponent.java#newcode150 chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsComponent.java:150: public void setOnComponentClosedHandler(OnComponentClosedHandler onComponentClosedHandler) ...
1 year, 3 months ago (2017-05-17 00:31:57 UTC) #19
thoren
https://codereview.chromium.org/2874943002/diff/40001/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsComponent.java File chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsComponent.java (right): https://codereview.chromium.org/2874943002/diff/40001/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsComponent.java#newcode150 chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsComponent.java:150: public void setOnComponentClosedHandler(OnComponentClosedHandler onComponentClosedHandler) { On 2017/05/17 00:31:56, Simeon ...
1 year, 3 months ago (2017-05-17 18:20:37 UTC) #21
halliwell
On 2017/05/17 18:20:37, thoren wrote: > https://codereview.chromium.org/2874943002/diff/40001/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsComponent.java > File > chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsComponent.java > (right): > > ...
1 year, 3 months ago (2017-05-19 16:45:54 UTC) #22
thoren
On 2017/05/19 16:45:54, halliwell wrote: > On 2017/05/17 18:20:37, thoren wrote: > > > https://codereview.chromium.org/2874943002/diff/40001/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsComponent.java ...
1 year, 3 months ago (2017-05-19 16:50:25 UTC) #25
halliwell
On 2017/05/19 16:50:25, thoren wrote: > On 2017/05/19 16:45:54, halliwell wrote: > > On 2017/05/17 ...
1 year, 3 months ago (2017-05-19 16:57:20 UTC) #26
halliwell
https://codereview.chromium.org/2874943002/diff/60001/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsActivity.java File chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsActivity.java (left): https://codereview.chromium.org/2874943002/diff/60001/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsActivity.java#oldcode95 chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsActivity.java:95: if (!CastBrowserHelper.initializeBrowser(getApplicationContext())) { What's the story behind removing this? ...
1 year, 3 months ago (2017-05-19 17:04:08 UTC) #27
thoren
https://codereview.chromium.org/2874943002/diff/60001/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsActivity.java File chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsActivity.java (left): https://codereview.chromium.org/2874943002/diff/60001/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsActivity.java#oldcode95 chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsActivity.java:95: if (!CastBrowserHelper.initializeBrowser(getApplicationContext())) { On 2017/05/19 17:04:08, halliwell wrote: > ...
1 year, 3 months ago (2017-05-19 17:19:50 UTC) #28
halliwell
Also please double check that internal build still works correctly on Fugu with this change. ...
1 year, 3 months ago (2017-05-19 18:14:06 UTC) #29
thoren
On 2017/05/19 18:14:06, halliwell wrote: > Also please double check that internal build still works ...
1 year, 2 months ago (2017-05-19 23:50:17 UTC) #36
halliwell
On 2017/05/19 23:50:17, thoren wrote: > On 2017/05/19 18:14:06, halliwell wrote: > > Also please ...
1 year, 2 months ago (2017-05-19 23:52:09 UTC) #37
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/2874943002/120001
1 year, 2 months ago (2017-05-22 16:34:38 UTC) #42
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/451210)
1 year, 2 months ago (2017-05-22 18:40:01 UTC) #44
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/2874943002/120001
1 year, 2 months ago (2017-05-22 19:00:44 UTC) #46
commit-bot: I haz the power
1 year, 2 months ago (2017-05-22 20:07:26 UTC) #49
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/9540c6d6f520ad49921f0dc4e6b9...

Powered by Google App Engine
This is Rietveld 408576698