Adds the ability for save dialogs to take a default extension....

Adds the ability for save dialogs to take a default extension.

BUG=4287
TEST=see bug


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

[M-A Ruel, 2008-11-12 01:15:25]

http://codereview.chromium.org/10621/diff/1/4
File chrome/browser/shell_dialogs.h (right):

http://codereview.chromium.org/10621/diff/1/4#newcode96
Line 96: void SelectFile(Type type,
I wonder who accepted this code in the first place. Function overloading, with
one being virtual. While at it, I'd say at least rename one of these functions.

http://codereview.chromium.org/10621/diff/1/2
File chrome/browser/views/bookmark_manager_view.cc (right):

http://codereview.chromium.org/10621/diff/1/2#newcode693
Line 693: const wchar_t filter_c_str[] = L"*.html\0*.html\0\0\0";
There's 2 extraneous \0 there. This string needs to be terminated with 2 NULL
and the mere fact that it is a string already adds one NULL. Plus, this string
is being copied on the stack at each invocation, that is inefficient. Please
rewrite as:
  const wchar_t* filter_c_str = L"*.html\0*.html\0";

[sky, 2008-11-12 16:09:09]

New snapshot pushed.

http://codereview.chromium.org/10621/diff/1/4
File chrome/browser/shell_dialogs.h (right):

http://codereview.chromium.org/10621/diff/1/4#newcode96
Line 96: void SelectFile(Type type,
On 2008/11/12 01:15:25, M-A wrote:
> I wonder who accepted this code in the first place. Function overloading, with
> one being virtual. While at it, I'd say at least rename one of these
functions.

You can blame me for this. I temporarily added the other to avoid updating all
callers. I've updated all callers now.

I was tempted to convert this function to take a struct given there are so many
args and it's damn near impossible to match things up. But that's for later.

[M-A Ruel, 2008-11-12 17:28:02]

lgtm, sorry for the delay.

http://codereview.chromium.org/10621/diff/601/17
File chrome/browser/views/bookmark_manager_view.cc (right):

http://codereview.chromium.org/10621/diff/601/17#newcode690
Line 690: static const wchar_t filter_c_str[] = L"*.html\0*.html\0";
Can you just name it kFilterStr so it's clear it's a constant?