Port some strings in download/save_package.cc

Port some strings in download/save_package.cc


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=9547

[tony, 2009-02-04 21:00:07]

LGTM

http://codereview.chromium.org/23004/diff/1004/2003
File chrome/browser/download/save_package.cc (right):

http://codereview.chromium.org/23004/diff/1004/2003#newcode950
Line 950: // It was copied from previous code but seems strange.
You could ask Johnny Ding (the original author) about it.  I think it's a bit
strange that it's an in/out param, but wasn't motivated enough to change it.

http://codereview.chromium.org/23004/diff/1004/2003#newcode961
Line 961: }
Nit: wrong indention?

http://codereview.chromium.org/23004/diff/1004/2003#newcode1019
Line 1019: right_paren_position = pure_file_name.rfind(kRightParen);
This looks fine from a porting perspective, but I think this code is fragile. 
What happens if there are parens in the file extension or if it's a filename
with surrogate pairs (in utf16) or if '(' or ')' is a byte in a utf8 filename?

You might want to add a TODO and file a bug about this.

http://codereview.chromium.org/23004/diff/1004/2004
File chrome/browser/download/save_package.h (right):

http://codereview.chromium.org/23004/diff/1004/2004#newcode219
Line 219: friend class SavePackageTest;
Nit: Can you change this to FRIEND_TEST(SavePackageTest, ...) while you're here.

[Evan Stade, 2009-02-04 23:19:31]

Addressed Tony's comments. Waiting for trybot unit test results.

I got rid of SavePackage::GetBaseFileNameAndFileOrdinalNumber because it  was
only used once and the file ordinal number was ignored.

[tony, 2009-02-04 23:24:27]

Removing GetBaseFileNameAndFileOrdinalNumber sounds good.  LGTM!

On 2009/02/04 23:19:31, estade wrote:
> Addressed Tony's comments. Waiting for trybot unit test results.
> 
> I got rid of SavePackage::GetBaseFileNameAndFileOrdinalNumber because it  was
> only used once and the file ordinal number was ignored.

[Evan Stade, 2009-02-05 01:30:37]

fyi, can't use FRIEND_TEST here because it's SavePackageTest and not
SavePackageTest_TestName that's using the private methods.