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

Issue 20110: Initial checkin of the HTML downloads UI. It's not yet complete (lacking... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years ago by Glen Murphy
Modified:
5 years, 7 months ago
Reviewers:
Paul Godavari, ojan
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Initial checkin of the HTML downloads UI. It's not yet complete (lacking interface polish). But is enough for us to begin removing the native UI.ojan, please review downloads.htmlpaul, please review everything else Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=10050

Patch Set 1 #

Total comments: 26

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Patch Set 5 : '' #

Patch Set 6 : '' #

Patch Set 7 : '' #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+1243 lines, -24 lines) Patch
M chrome/app/generated_resources.grd View 1 2 3 4 2 chunks +10 lines, -0 lines 0 comments Download
M chrome/browser/browser.vcproj View 1 2 3 4 1 chunk +16 lines, -0 lines 0 comments Download
M chrome/browser/browser_resources.grd View 5 6 1 chunk +23 lines, -22 lines 0 comments Download
M chrome/browser/dom_ui/dom_ui.h View 1 2 3 4 2 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/dom_ui/dom_ui.cc View 1 2 3 4 2 chunks +43 lines, -1 line 1 comment Download
M chrome/browser/dom_ui/dom_ui_contents.cc View 1 2 3 4 2 chunks +4 lines, -1 line 0 comments Download
A chrome/browser/dom_ui/downloads_ui.h View 1 2 1 chunk +120 lines, -0 lines 0 comments Download
A chrome/browser/dom_ui/downloads_ui.cc View 1 2 3 4 1 chunk +361 lines, -0 lines 0 comments Download
A chrome/browser/dom_ui/fileicon_source.h View 1 chunk +45 lines, -0 lines 0 comments Download
A chrome/browser/dom_ui/fileicon_source.cc View 1 chunk +63 lines, -0 lines 0 comments Download
A chrome/browser/resources/downloads.html View 1 2 3 4 5 1 chunk +551 lines, -0 lines 0 comments Download
Trybot results:
Commit queue not available (can’t edit this change).

Messages

Total messages: 6 (0 generated)
Glen Murphy
8 years ago (2009-02-05 23:21:58 UTC) #1
Paul Godavari
http://codereview.chromium.org/20110/diff/1/7 File chrome/browser/dom_ui/dom_ui.cc (right): http://codereview.chromium.org/20110/diff/1/7#newcode136 Line 136: static_cast<const StringValue*>(list_member); Nit: indent 2 more spaces. http://codereview.chromium.org/20110/diff/1/7#newcode139 ...
8 years ago (2009-02-06 00:16:27 UTC) #2
ojan
http://codereview.chromium.org/20110/diff/1/5 File chrome/browser/resources/downloads.html (right): http://codereview.chromium.org/20110/diff/1/5#newcode81 Line 81: /** Do we have a plan of any ...
8 years ago (2009-02-06 02:38:13 UTC) #3
Glen Murphy
Done, with the following exception notes: On 2009/02/06 02:38:13, paul wrote: > http://codereview.chromium.org/20110/diff/1/9#newcode45 > Line ...
8 years ago (2009-02-13 00:57:47 UTC) #4
ojan
LGTM
8 years ago (2009-02-13 01:48:31 UTC) #5
Paul Godavari
8 years ago (2009-02-17 22:23:18 UTC) #6
LGTM, with one nit.

http://codereview.chromium.org/20110/diff/54/1058
File chrome/browser/dom_ui/dom_ui.cc (right):

http://codereview.chromium.org/20110/diff/54/1058#newcode147
Line 147: return NULL;
Nit: I think this should return some default value here (like the empty string
in the method below) rather than returning NULL for an int value. Or you could
change the method signature to be:
bool Extract(const Value* in_value, int* out_int)
to better indicate success / failure.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f8e48bd