Issue 13183: Reupload of 12418

Issue 13183: Reupload of 12418 (Closed)

Created:
12 years ago by Glen Murphy

Modified:
9 years, 5 months ago

Reviewers:
ojan

CC:
chromium-reviews_googlegroups.com

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

Reupload of 12418 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=6497

Patch Set 1 #

Total comments: 3

Patch Set 2 : '' #

Created: 12 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+1850 lines, -230 lines)			Patch
M	chrome/app/generated_resources.grd	View	1	1 chunk	+46 lines, -15 lines	0 comments	Download
M	chrome/browser/browser.vcproj	View	1	2 chunks	+24 lines, -0 lines	0 comments	Download
M	chrome/browser/browser_about_handler.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/browser_main.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/browser_resources.h	View	1	1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/browser_resources.rc	View	1	1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/browser_url_handler.cc	View	1	2 chunks	+4 lines, -1 line	0 comments	Download
M	chrome/browser/browsing_instance.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/debugger/debugger_contents.cc	View	1	3 chunks	+3 lines, -3 lines	0 comments	Download
M	chrome/browser/debugger/debugger_view.cc	View	1	3 chunks	+3 lines, -4 lines	0 comments	Download
M	chrome/browser/dom_ui/chrome_url_data_manager.h	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/dom_ui/chrome_url_data_manager.cc	View	1	2 chunks	+3 lines, -3 lines	0 comments	Download
A	chrome/browser/dom_ui/dom_ui.h	View		1 chunk	+86 lines, -0 lines	0 comments	Download
A	chrome/browser/dom_ui/dom_ui.cc	View		1 chunk	+124 lines, -0 lines	0 comments	Download
A	chrome/browser/dom_ui/dom_ui_contents.h	View		1 chunk	+123 lines, -0 lines	0 comments	Download
A	chrome/browser/dom_ui/dom_ui_contents.cc	View		1 chunk	+207 lines, -0 lines	0 comments	Download
A	chrome/browser/dom_ui/history_ui.h	View		1 chunk	+83 lines, -0 lines	0 comments	Download
A	chrome/browser/dom_ui/history_ui.cc	View		1 chunk	+303 lines, -0 lines	0 comments	Download
M	chrome/browser/dom_ui/new_tab_ui.h	View	1	2 chunks	+3 lines, -68 lines	0 comments	Download
M	chrome/browser/dom_ui/new_tab_ui.cc	View	1	4 chunks	+2 lines, -102 lines	0 comments	Download
M	chrome/browser/history/history.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/native_ui_contents.h	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/render_view_context_menu_controller.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/renderer_security_policy.cc	View	1	2 chunks	+5 lines, -5 lines	0 comments	Download
M	chrome/browser/renderer_security_policy_unittest.cc	View	1	4 chunks	+6 lines, -6 lines	0 comments	Download
M	chrome/browser/resources/browser_resources.vcproj	View	1	1 chunk	+4 lines, -0 lines	0 comments	Download
A	chrome/browser/resources/history.html	View	1	1 chunk	+789 lines, -0 lines	0 comments	Download
M	chrome/browser/resources/new_tab.html	View	1	7 chunks	+9 lines, -9 lines	0 comments	Download
M	chrome/browser/site_instance.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/tab_contents_factory.cc	View	1	3 chunks	+7 lines, -0 lines	0 comments	Download
M	chrome/browser/tab_contents_type.h	View	1	1 chunk	+1 line, -0 lines	0 comments	Download
M	chrome/renderer/render_view.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/renderer/renderer_glue.cc	View	1	1 chunk	+2 lines, -2 lines	0 comments	Download
M	webkit/port/bindings/v8/v8_proxy.cpp	View	1	1 chunk	+0 lines, -1 line	0 comments	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

ojan

12 years ago (2008-12-05 20:58:04 UTC) #2

LGTM!

A couple nits. Feel free to ignore them if you disagree.

http://codereview.chromium.org/13183/diff/1/9
File chrome/browser/resources/history.html (right):

http://codereview.chromium.org/13183/diff/1/9#newcode113
Line 113: Page.prototype.getSearchResultHTML = function() {
No point in using an array unless you return the array and then do one big join
at the end.

http://codereview.chromium.org/13183/diff/1/9#newcode417
Line 417: this.pageIndex_ = parseInt(page);
parseInt(page, 0)

http://codereview.chromium.org/13183/diff/1/9#newcode497
Line 497: var navOutput = [];
No point in using an array here I think.

Expand Messages | Collapse Messages