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

Issue 12026010: Add GOOGLE_TV build tag (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 2 months ago by kjyoun
Modified:
2 years, 2 months ago
CC:
chromium-reviews, 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 #

Messages

Total messages: 25 (0 generated)
kjyoun
Can you start review this?
2 years, 2 months ago (2013-01-22 23:28:47 UTC) #1
Yaron
I assume there are more changes along these lines? Can you file a tracking bug ...
2 years, 2 months ago (2013-01-23 00:45:28 UTC) #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 ...
2 years, 2 months ago (2013-01-23 21:02:57 UTC) #3
kjyoun
I changed android_tv to google_tv, per Robert's recommendation BTW, though I changed my commit page, ...
2 years, 2 months ago (2013-01-23 23:33:39 UTC) #4
iannucci
On 2013/01/23 23:33:39, kjyoun wrote: > I changed android_tv to google_tv, > per Robert's recommendation ...
2 years, 2 months ago (2013-01-23 23:37:33 UTC) #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 ...
2 years, 2 months ago (2013-01-23 23:56:27 UTC) #6
Yaron
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 ...
2 years, 2 months ago (2013-01-24 00:32:29 UTC) #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 ...
2 years, 2 months ago (2013-01-24 00:55:46 UTC) #8
Yaron
lgtm but you need a webkit/OWNER
2 years, 2 months ago (2013-01-24 22:55:52 UTC) #9
kjyoun
On 2013/01/24 22:55:52, Yaron wrote: > lgtm but you need a webkit/OWNER Thanks for the ...
2 years, 2 months ago (2013-01-25 00:59:11 UTC) #10
kjyoun
Hi Brett/Darin Can you review my changes?
2 years, 2 months ago (2013-01-25 01:12:24 UTC) #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 ...
2 years, 2 months ago (2013-01-25 22:46:42 UTC) #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 ...
2 years, 2 months ago (2013-01-28 17:30:49 UTC) #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 ...
2 years, 2 months ago (2013-01-28 17:47:23 UTC) #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 ...
2 years, 2 months ago (2013-01-28 19:14:40 UTC) #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"
2 years, 2 months ago (2013-01-28 19:22:46 UTC) #16
brettw
LGTM from me, but please work out with John the best place for your media ...
2 years, 2 months ago (2013-01-28 19:27:25 UTC) #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 ...
2 years, 2 months ago (2013-01-28 22:04:27 UTC) #18
Yaron
lgtm
2 years, 2 months ago (2013-01-29 00:50:07 UTC) #19
kjyoun
Can you review the change? If, hopefully, there is no problem, can you merge this ...
2 years, 2 months ago (2013-01-30 21:49:53 UTC) #20
Yaron
On 2013/01/30 21:49:53, kjyoun wrote: > Can you review the change? > > If, hopefully, ...
2 years, 2 months ago (2013-01-30 23:21:43 UTC) #21
kjyoun
Um.. I could not find "Commit" checkbox. Where is it supposed to be located? Thanks
2 years, 2 months ago (2013-01-30 23:26:23 UTC) #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
2 years, 2 months ago (2013-01-30 23:26:29 UTC) #23
Yaron
On 2013/01/30 23:26:29, I haz the power (commit-bot) wrote: > CQ is trying da patch. ...
2 years, 2 months ago (2013-01-30 23:32:03 UTC) #24
I haz the power (commit-bot)
2 years, 2 months ago (2013-01-31 05:03:42 UTC) #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 700cc9d