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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10141005: switch ntp to jstemplate v2 (Closed)

Created:
8 years, 8 months ago by Evan Stade
Modified:
8 years, 7 months ago
Reviewers:
Dan Beam
CC:
chromium-reviews, dbeam+watch-ntp_chromium.org, estade+watch_chromium.org, arv (Not doing code reviews)
Visibility:
Public.

Description

switch ntp to jstemplate v2 BUG=122753, 124081, 124082, 124084 TEST=manual Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=134130

Patch Set 1 #

Patch Set 2 : update test #

Total comments: 19

Patch Set 3 : review #

Patch Set 4 : loc-- #

Patch Set 5 : rip out broken customlogo code #

Total comments: 1

Patch Set 6 : . #

Total comments: 2

Patch Set 7 : no curlies for you #

Patch Set 8 : fix test once and for all #

Unified diffs Side-by-side diffs Delta from patch set Stats (+218 lines, -217 lines) Patch
M chrome/app/theme/theme_resources.grd View 1 2 3 4 1 chunk +0 lines, -6 lines 0 comments Download
M chrome/browser/resources/ntp4/apps_page.js View 3 chunks +2 lines, -4 lines 0 comments Download
M chrome/browser/resources/ntp4/most_visited_page.js View 2 chunks +6 lines, -5 lines 0 comments Download
M chrome/browser/resources/ntp4/new_tab.html View 1 2 3 4 5 chunks +5 lines, -6 lines 0 comments Download
M chrome/browser/resources/ntp4/new_tab.js View 1 2 3 4 14 chunks +22 lines, -39 lines 0 comments Download
M chrome/browser/resources/ntp4/other_sessions.js View 2 chunks +1 line, -2 lines 0 comments Download
M chrome/browser/resources/ntp4/page_list_view.js View 14 chunks +22 lines, -28 lines 0 comments Download
M chrome/browser/resources/ntp4/recently_closed.js View 2 chunks +2 lines, -4 lines 0 comments Download
M chrome/browser/resources/ntp4/suggestions_page.js View 2 chunks +6 lines, -5 lines 0 comments Download
M chrome/browser/resources/shared/js/load_time_data.js View 1 2 3 4 5 6 3 chunks +43 lines, -4 lines 0 comments Download
M chrome/browser/ui/webui/chrome_web_ui_data_source.cc View 1 1 chunk +5 lines, -3 lines 0 comments Download
M chrome/browser/ui/webui/ntp/new_tab_page_handler.cc View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/webui/ntp/ntp_login_handler.cc View 1 chunk +7 lines, -7 lines 0 comments Download
M chrome/browser/ui/webui/ntp/ntp_resource_cache.cc View 1 2 3 4 2 chunks +58 lines, -81 lines 0 comments Download
M chrome/browser/ui/webui/ntp/ntp_resource_cache_android.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/common/common_resources.grd View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/jstemplate_builder.h View 2 chunks +14 lines, -3 lines 0 comments Download
M chrome/common/jstemplate_builder.cc View 5 chunks +21 lines, -15 lines 0 comments Download
chrome/test/data/webui/ntp4.js View 1 2 3 4 5 6 7 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Evan Stade
8 years, 8 months ago (2012-04-24 03:33:09 UTC) #1
Dan Beam
http://codereview.chromium.org/10141005/diff/8001/chrome/browser/resources/shared/js/load_time_data.js File chrome/browser/resources/shared/js/load_time_data.js (right): http://codereview.chromium.org/10141005/diff/8001/chrome/browser/resources/shared/js/load_time_data.js#newcode77 chrome/browser/resources/shared/js/load_time_data.js:77: getInteger: function(id) { Why getInteger() instead of getNumber()? http://codereview.chromium.org/10141005/diff/8001/chrome/browser/ui/webui/ntp/new_tab_page_handler.cc ...
8 years, 7 months ago (2012-04-25 23:13:12 UTC) #2
Evan Stade
https://chromiumcodereview.appspot.com/10141005/diff/8001/chrome/browser/resources/shared/js/load_time_data.js File chrome/browser/resources/shared/js/load_time_data.js (right): https://chromiumcodereview.appspot.com/10141005/diff/8001/chrome/browser/resources/shared/js/load_time_data.js#newcode77 chrome/browser/resources/shared/js/load_time_data.js:77: getInteger: function(id) { On 2012/04/25 23:13:12, Dan Beam wrote: ...
8 years, 7 months ago (2012-04-26 00:44:55 UTC) #3
Dan Beam
lgtm https://chromiumcodereview.appspot.com/10141005/diff/8001/chrome/browser/resources/shared/js/load_time_data.js File chrome/browser/resources/shared/js/load_time_data.js (right): https://chromiumcodereview.appspot.com/10141005/diff/8001/chrome/browser/resources/shared/js/load_time_data.js#newcode77 chrome/browser/resources/shared/js/load_time_data.js:77: getInteger: function(id) { On 2012/04/26 00:44:56, Evan Stade ...
8 years, 7 months ago (2012-04-26 01:41:29 UTC) #4
Dan Beam
https://chromiumcodereview.appspot.com/10141005/diff/22019/chrome/browser/resources/shared/js/load_time_data.js File chrome/browser/resources/shared/js/load_time_data.js (right): https://chromiumcodereview.appspot.com/10141005/diff/22019/chrome/browser/resources/shared/js/load_time_data.js#newcode91 chrome/browser/resources/shared/js/load_time_data.js:91: if (!condition) { nit: no curlies
8 years, 7 months ago (2012-04-26 01:42:38 UTC) #5
Evan Stade
https://chromiumcodereview.appspot.com/10141005/diff/22019/chrome/browser/resources/shared/js/load_time_data.js File chrome/browser/resources/shared/js/load_time_data.js (right): https://chromiumcodereview.appspot.com/10141005/diff/22019/chrome/browser/resources/shared/js/load_time_data.js#newcode91 chrome/browser/resources/shared/js/load_time_data.js:91: if (!condition) { On 2012/04/26 01:42:38, Dan Beam wrote: ...
8 years, 7 months ago (2012-04-26 01:46:18 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/estade@chromium.org/10141005/11004
8 years, 7 months ago (2012-04-26 03:07:02 UTC) #7
commit-bot: I haz the power
8 years, 7 months ago (2012-04-26 03:07:05 UTC) #8 
Can't process patch for file chrome/test/data/webui/ntp4.js.
File's status is None, patchset upload is incomplete.

Powered by Google App Engine
This is Rietveld 408576698