Chromium Code Reviews
Help | Chromium Project | Sign in
(629)

Issue 12026010: Add GOOGLE_TV build tag (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 2 months ago by kjyoun
Modified:
1 year, 2 months ago
CC:
chromium-reviews_chromium.org, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, googletv-nikita_google.com
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Add GOOGLE_TV build tag

add google_tv GYP variable for 'Android for Chrome' running on TV

BUG=172815
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=179798

Patch Set 1 #

Patch Set 2 : removed garbage line #

Patch Set 3 : remove webrtc to avoid build error #

Patch Set 4 : add missing header #

Total comments: 8

Patch Set 5 : addressed comments #

Patch Set 6 : rename android_tv to google_tv #

Patch Set 7 : change commit message #

Total comments: 4

Patch Set 8 : filed crbug and updated todo #

Total comments: 6

Patch Set 9 : #

Total comments: 4

Patch Set 10 : move a flag for EME under clank #

Patch Set 11 : rebased and add bug number #

Patch Set 12 : move ENABLE_EME under chrome #

Patch Set 13 : rebased and remove empry line #

Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -1 line) Lint Patch
M build/common.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +9 lines, -1 line 0 comments ? errors Download
M chrome/browser/android/chrome_startup_flags.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +4 lines, -0 lines 0 comments 0 errors Download
Commit:

Messages

Total messages: 25
kjyoun
Can you start review this?
1 year, 2 months ago #1
Yaron_ooo-April_21
I assume there are more changes along these lines? Can you file a tracking bug ...
1 year, 2 months ago #2
kjyoun
https://codereview.chromium.org/12026010/diff/6001/build/common.gypi File build/common.gypi (right): https://codereview.chromium.org/12026010/diff/6001/build/common.gypi#newcode113 build/common.gypi:113: # Sets whehther chrome is built for android tv ...
1 year, 2 months ago #3
kjyoun
I changed android_tv to google_tv, per Robert's recommendation BTW, though I changed my commit page, ...
1 year, 2 months ago #4
iannucci
On 2013/01/23 23:33:39, kjyoun wrote: > I changed android_tv to google_tv, > per Robert's recommendation ...
1 year, 2 months ago #5
iannucci
https://chromiumcodereview.appspot.com/12026010/diff/12005/build/common.gypi File build/common.gypi (right): https://chromiumcodereview.appspot.com/12026010/diff/12005/build/common.gypi#newcode533 build/common.gypi:533: ['(OS=="android" and google_tv!=1) or OS=="ios"', { why not just ...
1 year, 2 months ago #6
Yaron_ooo-April_21
https://codereview.chromium.org/12026010/diff/6001/content/browser/android/content_startup_flags.cc File content/browser/android/content_startup_flags.cc (right): https://codereview.chromium.org/12026010/diff/6001/content/browser/android/content_startup_flags.cc#newcode72 content/browser/android/content_startup_flags.cc:72: // Cdm plugin should run out of proces On ...
1 year, 2 months ago #7
kjyoun
https://codereview.chromium.org/12026010/diff/6001/content/browser/android/content_startup_flags.cc File content/browser/android/content_startup_flags.cc (right): https://codereview.chromium.org/12026010/diff/6001/content/browser/android/content_startup_flags.cc#newcode72 content/browser/android/content_startup_flags.cc:72: // Cdm plugin should run out of proces On ...
1 year, 2 months ago #8
Yaron_ooo-April_21
lgtm but you need a webkit/OWNER
1 year, 2 months ago #9
kjyoun
On 2013/01/24 22:55:52, Yaron wrote: > lgtm but you need a webkit/OWNER Thanks for the ...
1 year, 2 months ago #10
kjyoun
Hi Brett/Darin Can you review my changes?
1 year, 2 months ago #11
brettw
https://codereview.chromium.org/12026010/diff/9003/content/browser/android/content_startup_flags.cc File content/browser/android/content_startup_flags.cc (right): https://codereview.chromium.org/12026010/diff/9003/content/browser/android/content_startup_flags.cc#newcode72 content/browser/android/content_startup_flags.cc:72: // Cdm plugin should run out of process. Indenting ...
1 year, 2 months ago #12
kjyoun
https://codereview.chromium.org/12026010/diff/9003/content/browser/android/content_startup_flags.cc File content/browser/android/content_startup_flags.cc (right): https://codereview.chromium.org/12026010/diff/9003/content/browser/android/content_startup_flags.cc#newcode72 content/browser/android/content_startup_flags.cc:72: // Cdm plugin should run out of process. On ...
1 year, 2 months ago #13
jam
https://codereview.chromium.org/12026010/diff/18001/content/browser/android/content_startup_flags.cc File content/browser/android/content_startup_flags.cc (right): https://codereview.chromium.org/12026010/diff/18001/content/browser/android/content_startup_flags.cc#newcode72 content/browser/android/content_startup_flags.cc:72: parsed_command_line->AppendSwitch(switches::kEnableEncryptedMedia); can you do this somewhere under src/chrome? it's ...
1 year, 2 months ago #14
kjyoun
https://codereview.chromium.org/12026010/diff/18001/content/browser/android/content_startup_flags.cc File content/browser/android/content_startup_flags.cc (right): https://codereview.chromium.org/12026010/diff/18001/content/browser/android/content_startup_flags.cc#newcode72 content/browser/android/content_startup_flags.cc:72: parsed_command_line->AppendSwitch(switches::kEnableEncryptedMedia); Agreed, since this flag is googletv-specific. Then right ...
1 year, 2 months ago #15
kjyoun
https://codereview.chromium.org/12026010/diff/18001/content/browser/android/content_startup_flags.cc File content/browser/android/content_startup_flags.cc (right): https://codereview.chromium.org/12026010/diff/18001/content/browser/android/content_startup_flags.cc#newcode72 content/browser/android/content_startup_flags.cc:72: parsed_command_line->AppendSwitch(switches::kEnableEncryptedMedia); fixed typo: right location seems to be "src/clank/native/framework/chrome/chrome_main_delegate_app_android.cc"
1 year, 2 months ago #16
brettw
LGTM from me, but please work out with John the best place for your media ...
1 year, 2 months ago #17
kjyoun
https://codereview.chromium.org/12026010/diff/18001/content/browser/android/content_startup_flags.cc File content/browser/android/content_startup_flags.cc (right): https://codereview.chromium.org/12026010/diff/18001/content/browser/android/content_startup_flags.cc#newcode72 content/browser/android/content_startup_flags.cc:72: parsed_command_line->AppendSwitch(switches::kEnableEncryptedMedia); verified that the change on clank side can ...
1 year, 2 months ago #18
Yaron_ooo-April_21
lgtm
1 year, 2 months ago #19
kjyoun
Can you review the change? If, hopefully, there is no problem, can you merge this ...
1 year, 2 months ago #20
Yaron_ooo-April_21
On 2013/01/30 21:49:53, kjyoun wrote: > Can you review the change? > > If, hopefully, ...
1 year, 2 months ago #21
kjyoun
Um.. I could not find "Commit" checkbox. Where is it supposed to be located? Thanks
1 year, 2 months ago #22
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kjyoun@google.com/12026010/22004
1 year, 2 months ago #23
Yaron_ooo-April_21
On 2013/01/30 23:26:29, I haz the power (commit-bot) wrote: > CQ is trying da patch. ...
1 year, 2 months ago #24
I haz the power (commit-bot)
1 year, 2 months ago #25
Message was sent while issue was closed.
Change committed as 179798
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1275:d14800f88434