Allow POSIX callers to specify a new file's mode.

Allow POSIX callers to specify a new file's mode.

Retain the tight 0640 mode for most existing callers, but honor the user's
umask for downloaded files.

This is a re-land of https://codereview.chromium.org/639253004/.

BUG=409416, 362887
TBR=asanka,mdempsky,rvargas

Committed: https://crrev.com/931e35070097af9837f79ff19f8668e45b92ed3a
Cr-Commit-Position: refs/heads/master@{#302113}

[palmer, 2014-10-30 17:20:38]

The CQ bit was checked by palmer@chromium.org

[commit-bot: I haz the power, 2014-10-30 17:21:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/689713006/1

[commit-bot: I haz the power, 2014-10-30 18:29:31]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2014-10-30 18:30:13]

Patchset 1 (id:??) landed as
https://crrev.com/931e35070097af9837f79ff19f8668e45b92ed3a
Cr-Commit-Position: refs/heads/master@{#302113}

[csharp1, 2014-10-30 20:10:36]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/693493004/ by csharp@google.com.

The reason for reverting is: This seems to have broken the Linux MSan test bot.

[ RUN      ] FileUtilTest.WriteFileWithMode
../../base/files/file_util_unittest.cc:2164: Failure
Value of: status.st_mode & 0777
  Actual: 384
Expected: mode
Which is: 420
[  FAILED  ] FileUtilTest.WriteFileWithMode (1 ms).

[palmer, 2014-10-30 20:30:41]

On 2014/10/30 20:10:36, csharp1 wrote:
> A revert of this CL (patchset #1 id:1) has been created in
> https://codereview.chromium.org/693493004/ by mailto:csharp@google.com.
> 
> The reason for reverting is: This seems to have broken the Linux MSan test
bot.

Shouldn't that bot be in the CQ then?

What filesystem does that bot use?

[asanka, 2014-11-03 14:44:57]

asanka@chromium.org changed reviewers:
+ asanka@chromium.org

[asanka, 2014-11-03 14:44:58]

https://codereview.chromium.org/689713006/diff/1/base/files/file_util_unittes...
File base/files/file_util_unittest.cc (right):

https://codereview.chromium.org/689713006/diff/1/base/files/file_util_unittes...
base/files/file_util_unittest.cc:2152: if (PathExists(data_dir)) {
This test, and the creation of a separate test directory should not be necessary
since it implies that temp_dir_ is not unique. The test fixture is supposed to
guarantee that it is unique per test. I.e. each individual test is given a
freshly constructed fixture including its own temp directory.

https://codereview.chromium.org/689713006/diff/1/base/files/file_util_unittes...
base/files/file_util_unittest.cc:2163: ASSERT_EQ(0, stat(foobar.value().c_str(),
&status));
Is there's a more aggressive umask on the MSan bots? If the umask is something
like 066, then we could end up with the results we are seeing. I can't tell for
sure without knowing what the configuration there is.

I agree with you that unless there's a memory access error or something, a test
that passes on the Linux bot should pass on the Linux MSan bot. But I also think
it might be best for the test to account for non-standard umasks, either by
explicitly setting a umask, or taking the umask into account.

This has to be done carefully, since if the test is not running in its own
process, then it is running in series with other tests. So we have to restore
the umask to its former glory before returning.

I prefer setting an explicit umask since otherwise the test wouldn't be testing
anything if the umask in effect is overly aggressive.