Win: Don't give some downloads duplicate extensions like .arj.arj...

Win: Don't give some downloads duplicate extensions like .arj.arj

BUG=49621
TEST=See bug. Also unittests.


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

[Nico, 2010-08-02 21:01:40]

This is maybe more complicated than it has to be, but since this is going into
m6 I'd like to play it safe.

[Nico, 2010-08-02 21:42:22]

Updated as per in-person review comments.

Do you have an example where the new code is better than the old code? The tests
didn't find a difference -- we should probably add a test case that explains why
the difference is important.

[Peter Kasting, 2010-08-02 21:59:01]

LGTM

http://codereview.chromium.org/3069018/diff/8001/9001
File chrome/browser/views/shell_dialogs_win.cc (right):

http://codereview.chromium.org/3069018/diff/8001/9001#newcode45
chrome/browser/views/shell_dialogs_win.cc:45: // If the user didn't give us a
known extension, and we wanted one, add it.
Nit: This comment might make more sense as:
// If we wanted a specific extension, but the user's filename deleted it or
changed it to something that the system doesn't understand, re-append it.

[Nico, 2010-08-02 22:14:54]

I also added a test that shows why this is different from patch set 1 in
http://codereview.chromium.org/2825010 (admittedly nothing that's likely to
happen in practice).

Then I added a test that checks that issue 7499, which weinjared wanted to fix,
is actually fixed -- it fails. I'm now confused. (Test:

TEST(AppendExtensionIfNeeded, EqualToExtension_UserCanOverrideExtension) {
  const std::wstring filename = L"product.reg";
  const std::wstring filter_selected = L"*.txt";
  const std::wstring suggested_ext = L"txt";

  const std::wstring actual_filename = AppendExtensionIfNeeded(filename,
      filter_selected, suggested_ext);

  ASSERT_EQ(L"product.reg", actual_filename);
}
)

Anyway, this CL fixes what I set out to fix, so I'll submit it and call it a
day.

http://codereview.chromium.org/3069018/diff/8001/9001
File chrome/browser/views/shell_dialogs_win.cc (right):

http://codereview.chromium.org/3069018/diff/8001/9001#newcode45
chrome/browser/views/shell_dialogs_win.cc:45: // If the user didn't give us a
known extension, and we wanted one, add it.
On 2010/08/02 21:59:01, Peter Kasting wrote:
> Nit: This comment might make more sense as:
> // If we wanted a specific extension, but the user's filename deleted it or
> changed it to something that the system doesn't understand, re-append it.

Done.