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

Issue 27106: Get save page working on posix. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years ago by Evan Stade
Modified:
5 years, 8 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Get save page working on posix. * rearrange some IFDEFs * trim whitespace on suggest name Also fix a relatively new bug in DomSerializer. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=10314

Patch Set 1 #

Total comments: 2

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -18 lines) Patch
M chrome/browser/browser.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/browser.cc View 3 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/download/save_package.cc View 1 7 chunks +4 lines, -8 lines 0 comments Download
M webkit/glue/dom_serializer.cc View 2 chunks +6 lines, -3 lines 0 comments Download
Trybot results:
Commit queue not available (can’t edit this change).

Messages

Total messages: 4 (0 generated)
Evan Stade
note that trimming the whitespace on the suggested file name will not affect windows (except ...
8 years ago (2009-02-24 22:58:45 UTC) #1
brettw (plz ping after 24h)
LGTM with one change http://codereview.chromium.org/27106/diff/1/4 File chrome/browser/download/save_package.cc (right): http://codereview.chromium.org/27106/diff/1/4#newcode936 Line 936: DCHECK(download_); I don't think ...
8 years ago (2009-02-24 23:06:12 UTC) #2
tony
LGTM2. On 2009/02/24 23:06:12, brettw wrote: > LGTM with one change > > http://codereview.chromium.org/27106/diff/1/4 > ...
8 years ago (2009-02-24 23:07:24 UTC) #3
Evan Stade
8 years ago (2009-02-24 23:19:36 UTC) #4
http://codereview.chromium.org/27106/diff/1/4
File chrome/browser/download/save_package.cc (right):

http://codereview.chromium.org/27106/diff/1/4#newcode936
Line 936: DCHECK(download_);
On 2009/02/24 23:06:12, brettw wrote:
> I don't think its useful to DCHECK for NULL pointers in cases like this, since
> it will immediately crash on the next line with a really obvious NULL pointer
> exception even in release mode. So I would remove this addition.

I added it for consistency because there are tons of places in this file where
we DCHECK(download_) right before dereferencing it. Shall I remove the other
DCHECKS as well or just this one?
Sign in to reply to this message.

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