Add File::Duplicate to duplicate a file handle.

Add File::Duplicate to duplicate a file handle.

BUG=462584
R=rvargas@chromium.org

Committed: https://crrev.com/bad5603dd8bbcf7583e8e483f8198e9b88f1c079
Cr-Commit-Position: refs/heads/master@{#321389}

[grt (UTC plus 2), 2015-03-18 19:41:25]

Hi Ricardo. I think this is in-line with our discussion via email. What do you
think? I'm trusting the bots to test the POSIX code for me, so don't be alarmed
if there's redness there. I'll deal with it if so. Thanks for taking a look.

[rvargas (doing something else), 2015-03-18 20:56:43]

Looks fine

https://codereview.chromium.org/1017243002/diff/20001/base/files/file.h
File base/files/file.h (right):

https://codereview.chromium.org/1017243002/diff/20001/base/files/file.h#newco...
base/files/file.h:290: // Returns a new handle to this file for use within the
current process. On
nit: handle -> object

https://codereview.chromium.org/1017243002/diff/20001/base/files/file.h#newco...
base/files/file.h:291: // POSIX, only the original File will obey
FLAG_DELETE_ON_CLOSE.
I don't think this behavior is posix exclusive... or rather, the comment is kind
of misleading about what happens. How about saying that on posix that flag is
not propagated (if we have to say something here)

https://codereview.chromium.org/1017243002/diff/20001/base/files/file_unittes...
File base/files/file_unittest.cc (right):

https://codereview.chromium.org/1017243002/diff/20001/base/files/file_unittes...
base/files/file_unittest.cc:486: // Write through one handle, close it, read
through the other.
We should be testing delete on close, not the whole thing we tested on the
previous test.

https://codereview.chromium.org/1017243002/diff/20001/base/files/file_win.cc
File base/files/file_win.cc (right):

https://codereview.chromium.org/1017243002/diff/20001/base/files/file_win.cc#...
base/files/file_win.cc:326: return File(OSErrorToFileError(last_error));
nit: inline last_error

[grt (UTC plus 2), 2015-03-18 21:06:25]

https://codereview.chromium.org/1017243002/diff/20001/base/files/file.h
File base/files/file.h (right):

https://codereview.chromium.org/1017243002/diff/20001/base/files/file.h#newco...
base/files/file.h:291: // POSIX, only the original File will obey
FLAG_DELETE_ON_CLOSE.
On 2015/03/18 20:56:42, rvargas wrote:
> I don't think this behavior is posix exclusive... or rather, the comment is
kind
> of misleading about what happens. How about saying that on posix that flag is
> not propagated (if we have to say something here)

I singled out POSIX here because "delete on close" is implemented by base::File
there, whereas on Windows the OS does it and the order of closes doesn't matter:
the deletion takes place when all are gone (at least, this is my understanding).
How does this sound:

Handling of FLAG_DELETE_ON_CLOSE varies by OS. On POSIX, the File object that
was created or initialized with this flag will unlink the underlying file on
close. On Windows, the underlying file is deleted when the last handle to it is
closed.

[rvargas (doing something else), 2015-03-18 21:29:58]

https://codereview.chromium.org/1017243002/diff/20001/base/files/file.h
File base/files/file.h (right):

https://codereview.chromium.org/1017243002/diff/20001/base/files/file.h#newco...
base/files/file.h:291: // POSIX, only the original File will obey
FLAG_DELETE_ON_CLOSE.
On 2015/03/18 21:06:24, grt wrote:
> On 2015/03/18 20:56:42, rvargas wrote:
> > I don't think this behavior is posix exclusive... or rather, the comment is
> kind
> > of misleading about what happens. How about saying that on posix that flag
is
> > not propagated (if we have to say something here)
> 
> I singled out POSIX here because "delete on close" is implemented by
base::File
> there, whereas on Windows the OS does it and the order of closes doesn't
matter:
> the deletion takes place when all are gone (at least, this is my
understanding).
> How does this sound:
> 
> Handling of FLAG_DELETE_ON_CLOSE varies by OS. On POSIX, the File object that
> was created or initialized with this flag will unlink the underlying file on
> close. On Windows, the underlying file is deleted when the last handle to it
is
> closed.

sounds good

[grt (UTC plus 2), 2015-03-19 03:31:03]

PTAL. Please feel at liberty to check the CQ box if this looks good to you.
Thanks.

https://codereview.chromium.org/1017243002/diff/20001/base/files/file.h
File base/files/file.h (right):

https://codereview.chromium.org/1017243002/diff/20001/base/files/file.h#newco...
base/files/file.h:290: // Returns a new handle to this file for use within the
current process. On
On 2015/03/18 20:56:42, rvargas wrote:
> nit: handle -> object

Done.

https://codereview.chromium.org/1017243002/diff/20001/base/files/file.h#newco...
base/files/file.h:291: // POSIX, only the original File will obey
FLAG_DELETE_ON_CLOSE.
On 2015/03/18 21:29:58, rvargas wrote:
> On 2015/03/18 21:06:24, grt wrote:
> > On 2015/03/18 20:56:42, rvargas wrote:
> > > I don't think this behavior is posix exclusive... or rather, the comment
is
> > kind
> > > of misleading about what happens. How about saying that on posix that flag
> is
> > > not propagated (if we have to say something here)
> > 
> > I singled out POSIX here because "delete on close" is implemented by
> base::File
> > there, whereas on Windows the OS does it and the order of closes doesn't
> matter:
> > the deletion takes place when all are gone (at least, this is my
> understanding).
> > How does this sound:
> > 
> > Handling of FLAG_DELETE_ON_CLOSE varies by OS. On POSIX, the File object
that
> > was created or initialized with this flag will unlink the underlying file on
> > close. On Windows, the underlying file is deleted when the last handle to it
> is
> > closed.
> 
> sounds good

Done.

https://codereview.chromium.org/1017243002/diff/20001/base/files/file_unittes...
File base/files/file_unittest.cc (right):

https://codereview.chromium.org/1017243002/diff/20001/base/files/file_unittes...
base/files/file_unittest.cc:486: // Write through one handle, close it, read
through the other.
On 2015/03/18 20:56:42, rvargas wrote:
> We should be testing delete on close, not the whole thing we tested on the
> previous test.

Done.

https://codereview.chromium.org/1017243002/diff/20001/base/files/file_win.cc
File base/files/file_win.cc (right):

https://codereview.chromium.org/1017243002/diff/20001/base/files/file_win.cc#...
base/files/file_win.cc:326: return File(OSErrorToFileError(last_error));
On 2015/03/18 20:56:42, rvargas wrote:
> nit: inline last_error

Done.

[grt (UTC plus 2), 2015-03-19 04:07:10]

Hmm...

https://codereview.chromium.org/1017243002/diff/40001/base/files/file_unittes...
File base/files/file_unittest.cc (right):

https://codereview.chromium.org/1017243002/diff/40001/base/files/file_unittes...
base/files/file_unittest.cc:482: ASSERT_TRUE(base::PathExists(file_path));
This fails on ios_dbg_simulator_ninja. Is this expected to not work there? I
notice that other tests in this file don't use PathExists after opening a file
to make sure it exists. Should I simply remove this line, #if !defined(OS_IOS)
around it, or something else?

[grt (UTC plus 2), 2015-03-19 15:26:27]

I think this one should work. Please flit the CQ bit if you approve. Thanks.

https://codereview.chromium.org/1017243002/diff/40001/base/files/file_unittes...
File base/files/file_unittest.cc (right):

https://codereview.chromium.org/1017243002/diff/40001/base/files/file_unittes...
base/files/file_unittest.cc:482: ASSERT_TRUE(base::PathExists(file_path));
On 2015/03/19 04:07:09, grt wrote:
> This fails on ios_dbg_simulator_ninja. Is this expected to not work there? I
> notice that other tests in this file don't use PathExists after opening a file
> to make sure it exists. Should I simply remove this line, #if !defined(OS_IOS)
> around it, or something else?

Ah, I'd misunderstood how DELETE_ON_CLOSE worked on posix. I've fixed the test
and the doc comment in files.h.

[rvargas (doing something else), 2015-03-19 17:48:45]

lgtm

[rvargas (doing something else), 2015-03-19 17:48:49]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2015-03-19 17:49:22]

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

[commit-bot: I haz the power, 2015-03-19 17:54:49]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-19 17:55:27]

Patchset 4 (id:??) landed as
https://crrev.com/bad5603dd8bbcf7583e8e483f8198e9b88f1c079
Cr-Commit-Position: refs/heads/master@{#321389}