Add FileHandleFileWriter

Add WeakFileHandleFileWriter, a FileWriterInterface implementation that
deals solely with a weak FileHandle.

CrashReportDatabase::PrepareNewCrashReport() provides its caller with
both a FileHandle and a FilePath. While it’s possible to create a
FileWriter from the FilePath, it’s not necessary to have two FileHandles
open to the same file. Also, there’s no FileWriteMode::kReuseOrFail
option because it didn’t seem necessary[1], and although it would
actually be the most desirable option for a FileWriter here, allowing
the FileHandle to be used directly without reopening the file sidesteps
the problem entirely.

FileWriter is adapted to use WeakFileHandleFileWriter to minimize
duplication.

[1] https://codereview.chromium.org/818433002/diff/80001/util/file/file_io.h#newcode138

R=rsesek@chromium.org, scottmg@chromium.org

Committed: https://chromium.googlesource.com/crashpad/crashpad/+/222f91f5c6e875929fabc00c6ba37698a2808172

[Mark Mentovai, 2015-02-04 21:02:22]

mark@chromium.org changed reviewers:
+ rsesek@chromium.org, scottmg@chromium.org

[scottmg, 2015-02-04 21:14:10]

Code LGTM, but the name doesn't really communicate the non-opening/closing-ness
to me.

I can't think of a much better one though. NonOwningFileWriter?

https://codereview.chromium.org/871193010/diff/1/util/file/file_writer.h
File util/file/file_writer.h (right):

https://codereview.chromium.org/871193010/diff/1/util/file/file_writer.h#newc...
util/file/file_writer.h:112: // FileHandleFileWriter  with a correct file
descriptor at the time of
extra space here

[Mark Mentovai, 2015-02-04 21:17:57]

It varies from FileWriter in two ways: the non-owning behavior and the fact that
it takes FileHandles instead of FilePaths. FileHandleFileWriter communicates one
distinction, and NonOwningFileWriter communicates the other.

NonOwningFileHandleFileWriter seems kind of wordy. WeakFileHandleFileWriter?

[scottmg, 2015-02-04 21:20:34]

On 2015/02/04 21:17:57, Mark Mentovai wrote:
> It varies from FileWriter in two ways: the non-owning behavior and the fact
that
> it takes FileHandles instead of FilePaths. FileHandleFileWriter communicates
one
> distinction, and NonOwningFileWriter communicates the other.
> 
> NonOwningFileHandleFileWriter seems kind of wordy. WeakFileHandleFileWriter?

SGTM if you don't think it's too long.

[Robert Sesek, 2015-02-04 21:25:01]

LGTM, maybe a test, but since it's already implicitly tested as part of the
implementation, not necessary.

[Mark Mentovai, 2015-02-04 21:29:55]

New patchsets have been uploaded after l-g-t-m from
scottmg@chromium.org,rsesek@chromium.org

[Mark Mentovai, 2015-02-04 21:31:17]

Patchset #2 (id:20001) has been deleted

[Mark Mentovai, 2015-02-04 21:32:40]

Renamed. WeakFileHandleFileWriter is wordy, but slightly better than
NonOwningFileHandleFileWriter.

No new test for now, this is as tested as FileWriter was.

[Robert Sesek, 2015-02-04 22:15:47]

Still LGTM

[Mark Mentovai, 2015-02-04 22:26:22]

Committed patchset #2 (id:40001) manually as
222f91f5c6e875929fabc00c6ba37698a2808172 (presubmit successful).