Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(88)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1390023002: Add FileWriteMode::kCreateOrFail (Closed)

Created:
5 years, 2 months ago by Mark Mentovai
Modified:
5 years, 2 months ago
Reviewers:
scottmg
CC:
crashpad-dev_chromium.org
Base URL:
https://chromium.googlesource.com/crashpad/crashpad@master
Target Ref:
refs/heads/master
Project:
crashpad
Visibility:
Public.

Description

Add FileWriteMode::kCreateOrFail BUG=crashpad:63 TEST=crashpad_util_test FileIO.OpenFileForWrite R=scottmg@chromium.org Committed: https://chromium.googlesource.com/crashpad/crashpad/+/2d8a0498ab03f646198be770e417365540372ea8

Patch Set 1 #

Patch Set 2 : CrashReportDatabaseTest too #

Patch Set 3 : Self-review #

Total comments: 1

Patch Set 4 : Better test #

Patch Set 5 : Test that kReuseOrCreate can create #

Unified diffs Side-by-side diffs Delta from patch set Stats (+233 lines, -51 lines) Patch
M client/crash_report_database_test.cc View 1 9 chunks +10 lines, -23 lines 0 comments Download
A test/file.h View 1 chunk +45 lines, -0 lines 0 comments Download
A test/file.cc View 1 2 1 chunk +66 lines, -0 lines 0 comments Download
M test/scoped_temp_dir_test.cc View 1 2 2 chunks +1 line, -22 lines 0 comments Download
M test/test.gyp View 1 chunk +2 lines, -0 lines 0 comments Download
M util/file/file_io.h View 1 chunk +3 lines, -0 lines 0 comments Download
M util/file/file_io_posix.cc View 1 chunk +16 lines, -6 lines 0 comments Download
M util/file/file_io_test.cc View 1 2 3 4 1 chunk +87 lines, -0 lines 0 comments Download
M util/file/file_io_win.cc View 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (1 generated)
Mark Mentovai
This supports bug crashpad:63, but doesn’t resolve it.
5 years, 2 months ago (2015-10-06 22:57:07 UTC) #2
scottmg
lgtm https://codereview.chromium.org/1390023002/diff/40001/util/file/file_io_test.cc File util/file/file_io_test.cc (right): https://codereview.chromium.org/1390023002/diff/40001/util/file/file_io_test.cc#newcode72 util/file/file_io_test.cc:72: file_handle.reset(LoggingOpenFileForWrite(file_path_1, What does doing this a second time ...
5 years, 2 months ago (2015-10-06 23:22:02 UTC) #3
Mark Mentovai
You’re right, the test was kind of dumb about a couple of things. I updated ...
5 years, 2 months ago (2015-10-07 01:06:17 UTC) #4
scottmg
LGTM
5 years, 2 months ago (2015-10-07 05:19:35 UTC) #5
Mark Mentovai
5 years, 2 months ago (2015-10-07 12:20:59 UTC) #6
Message was sent while issue was closed. 
Committed patchset #5 (id:80001) manually as
2d8a0498ab03f646198be770e417365540372ea8 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698