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

Issue 12508004: Enabling testing Google Now component extension (Closed)

Created:
7 years, 9 months ago by vadimt
Modified:
7 years, 9 months ago
CC:
chromium-reviews, arv+watch_chromium.org, govind1, stromme
Visibility:
Public.

Description

Enabling testing Google Now component extension. Google Now component extension now can be turned on via chrome://flags. The server name needs to be set manually in JS debugger via local memory. BUG=164227 TEST=Enable the extension via chrome://flags, set the server via local memory, restart, make sure cards appear. Check this on CrOS and Windows. When this starts working in Canary, set it up to see cards, wait till next day, update Canary to the next version, make sure cards appear on (1) the first start after the update; (2) following starts. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=187341

Patch Set 1 #

Total comments: 12

Patch Set 2 : rgustafson's notes #

Total comments: 7

Patch Set 3 : skare@'s notes #

Patch Set 4 : #

Patch Set 5 : Rebase #

Patch Set 6 : #

Total comments: 2

Patch Set 7 : Removing unnecessary workaround. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -11 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +7 lines, -0 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 4 5 1 chunk +9 lines, -0 lines 0 comments Download
M chrome/browser/chrome_browser_main.cc View 1 2 3 4 1 chunk +0 lines, -8 lines 0 comments Download
M chrome/browser/resources/google_now/background.js View 1 2 3 4 5 6 1 chunk +3 lines, -3 lines 0 comments Download

Messages

Total messages: 15 (0 generated)
vadimt
7 years, 9 months ago (2013-03-06 01:42:08 UTC) #1
rgustafson
https://codereview.chromium.org/12508004/diff/1/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/12508004/diff/1/chrome/app/generated_resources.grd#newcode15043 chrome/app/generated_resources.grd:15043: + Enable Google Now intergation integration https://codereview.chromium.org/12508004/diff/1/chrome/app/generated_resources.grd#newcode15046 chrome/app/generated_resources.grd:15046: + ...
7 years, 9 months ago (2013-03-06 02:51:57 UTC) #2
vadimt
https://codereview.chromium.org/12508004/diff/1/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/12508004/diff/1/chrome/app/generated_resources.grd#newcode15043 chrome/app/generated_resources.grd:15043: + Enable Google Now intergation On 2013/03/06 02:51:57, rgustafson ...
7 years, 9 months ago (2013-03-06 21:44:40 UTC) #3
skare_
https://codereview.chromium.org/12508004/diff/6001/chrome/browser/chrome_browser_main.cc File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/12508004/diff/6001/chrome/browser/chrome_browser_main.cc#newcode1061 chrome/browser/chrome_browser_main.cc:1061: // switches::kEnableGoogleNowIntegration)) { can this block just be removed ...
7 years, 9 months ago (2013-03-06 22:08:01 UTC) #4
vadimt
https://codereview.chromium.org/12508004/diff/6001/chrome/browser/chrome_browser_main.cc File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/12508004/diff/6001/chrome/browser/chrome_browser_main.cc#newcode1061 chrome/browser/chrome_browser_main.cc:1061: // switches::kEnableGoogleNowIntegration)) { On 2013/03/06 22:08:01, Travis Skare wrote: ...
7 years, 9 months ago (2013-03-06 22:37:12 UTC) #5
rgustafson
lgtm https://codereview.chromium.org/12508004/diff/1/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/12508004/diff/1/chrome/browser/about_flags.cc#newcode1270 chrome/browser/about_flags.cc:1270: kOsDesktop, Thanks. On 2013/03/06 21:44:40, vadimt wrote: > ...
7 years, 9 months ago (2013-03-06 22:56:00 UTC) #6
skare_
https://codereview.chromium.org/12508004/diff/6001/chrome/browser/chrome_browser_main.cc File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/12508004/diff/6001/chrome/browser/chrome_browser_main.cc#newcode1061 chrome/browser/chrome_browser_main.cc:1061: // switches::kEnableGoogleNowIntegration)) { 1) consider an actual bug for ...
7 years, 9 months ago (2013-03-07 00:56:39 UTC) #7
vadimt
https://codereview.chromium.org/12508004/diff/6001/chrome/browser/chrome_browser_main.cc File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/12508004/diff/6001/chrome/browser/chrome_browser_main.cc#newcode1061 chrome/browser/chrome_browser_main.cc:1061: // switches::kEnableGoogleNowIntegration)) { On 2013/03/07 00:56:39, Travis Skare wrote: ...
7 years, 9 months ago (2013-03-07 22:33:07 UTC) #8
skare_
lgtm great, thanks for removing that block
7 years, 9 months ago (2013-03-07 22:57:16 UTC) #9
vadimt
Please provide owners' approvals: sky@: *.cc; *.grd arv@: *.js NOTE: The CL also fixed a ...
7 years, 9 months ago (2013-03-07 23:41:40 UTC) #10
sky
LGTM
7 years, 9 months ago (2013-03-08 00:44:38 UTC) #11
arv (Not doing code reviews)
LGTM with nit https://codereview.chromium.org/12508004/diff/30001/chrome/browser/resources/google_now/background.js File chrome/browser/resources/google_now/background.js (right): https://codereview.chromium.org/12508004/diff/30001/chrome/browser/resources/google_now/background.js#newcode111 chrome/browser/resources/google_now/background.js:111: if (items.activeNotifications.hasOwnProperty(notificationId) && Does it matter ...
7 years, 9 months ago (2013-03-11 16:04:21 UTC) #12
vadimt
https://codereview.chromium.org/12508004/diff/30001/chrome/browser/resources/google_now/background.js File chrome/browser/resources/google_now/background.js (right): https://codereview.chromium.org/12508004/diff/30001/chrome/browser/resources/google_now/background.js#newcode111 chrome/browser/resources/google_now/background.js:111: if (items.activeNotifications.hasOwnProperty(notificationId) && On 2013/03/11 16:04:21, arv wrote: > ...
7 years, 9 months ago (2013-03-11 16:51:02 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vadimt@chromium.org/12508004/11006
7 years, 9 months ago (2013-03-11 16:51:27 UTC) #14
commit-bot: I haz the power
7 years, 9 months ago (2013-03-11 19:36:16 UTC) #15
Message was sent while issue was closed.
Change committed as 187341

Powered by Google App Engine
This is Rietveld 408576698