Don't expand path to use long names in CreateTemporaryFileInDir.

[Win] Don't call GetLongPathName() in CreateTemporaryFileInDir().

GetLongPathName() can fail if the user doesn't have the necessary
privileges on an ancestor directory. If this call failed during
CreateTemporaryFileInDir(), the temporary file that was created
in the previous GetTempFileName() call will leak.

None of the callers rely on the path being expanded on return. So
get rid of the call.

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

[asanka, 2012-11-06 22:32:29]

brettw: I choose you due to http://b/issue?id=595243 . It's only 6 years
old. I'm sure you remember all the details :)

On Windows file_util::CreateTemporaryFileInDir() calls GetTempFileName() to
create a temporary file, and then calls GetLongPathName() to get a path with
long names. This GetLongPathName() call has two problems:

1. As the bug indicates, GetLongPathName() doesn't always work. In particular,
   if the user doesn't have the necessary permissions on any ancestor directory,
   the call will fail.

2. If the GetLongPathName() call fails, so does CreateTemporaryFileInDir(). But
   the temporary file that was created successfully during the GetTempFileName()
   call is now left around without being cleaned up.

The CLs that introduced this logic is
https://gerrit-int.chromium.org/gitweb?p=chrome/chrome-history.git;a=commit;h...
and
https://gerrit-int.chromium.org/gitweb?p=chrome/chrome-history.git;a=commit;h....

AFAICT these CLs don't depend on the returned path not having any short names.

Currently there are many callers of CreateTemporaryFileInDir() and
CreateTemporaryFile(), none of whom depend on the returned paths not having
short names. It also seems like an unexpected side-effect of
CreateTemporaryFileInDir() that the returned path will be different from the
|dir| parameter that's passed in. In addition, CreateTemporaryDirInDir() doesn't
have corresponding behavior.

Hence this CL. What do you think?

These problems came up recently because we started creating the initial
temporary file for a download in the user's preferred downloads directory
instead of in %TMP%.

[brettw, 2012-11-06 22:46:13]

I defer my review to Carlos. "LGTM" to make the CQ owners check happy.

[asanka, 2012-11-08 22:52:56]

On 2012/11/06 22:46:13, brettw wrote:
> I defer my review to Carlos. "LGTM" to make the CQ owners check happy.

Thanks!

cpu: ping?

[cpu_(ooo_6.6-7.5), 2012-11-09 17:21:47]

Sorry for the delay I am on vacation.

http://codereview.chromium.org/11300011/diff/7001/base/file_util_win.cc
File base/file_util_win.cc (right):

http://codereview.chromium.org/11300011/diff/7001/base/file_util_win.cc#newco...
base/file_util_win.cc:396: *temp_file = FilePath(temp_name);
In principle I agree with you assessment but I would still like to have the
expanded path whenever the system allows it. So I think is best to keep the call
but don't make it fatal, and also remove the log statement.

[asanka, 2012-11-09 22:02:54]

Restored the the GetLongPathName() call, and added a test.

http://codereview.chromium.org/11300011/diff/10002/base/file_util_win.cc
File base/file_util_win.cc (right):

http://codereview.chromium.org/11300011/diff/10002/base/file_util_win.cc#newc...
base/file_util_win.cc:396: wchar_t long_temp_name[MAX_PATH + 1];
Added |long_temp_name| because I couldn't find any guarantees that
GetLongPathName() would leave the destination buffer untouched on failure.

[cpu_(ooo_6.6-7.5), 2012-11-09 22:47:01]

lgtm

[asanka, 2012-11-11 04:43:54]

On 2012/11/09 22:47:01, cpu wrote:
> lgtm

Thank you!

[commit-bot: I haz the power, 2012-11-12 15:02:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asanka@chromium.org/11300011/10002

[commit-bot: I haz the power, 2012-11-12 16:28:11]

Change committed as 167189