Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(22)

Issue 1637433003: Remove use of deprecated CFURLCreateDataAndPropertiesFromResource function. (Closed)

Created:
4 years, 10 months ago by ivanpe
Modified:
4 years, 10 months ago
CC:
google-breakpad-dev_googlegroups.com, Olivier
Base URL:
https://chromium.googlesource.com/breakpad/breakpad.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Remove use of deprecated CFURLCreateDataAndPropertiesFromResource function. Original change (https://codereview.chromium.org/1527363003/) was failing in CFReadStreamGetBuffer() call, so changed to CFReadStreamRead() to be more conservative. Patch provided by Scott Hancher. BUG= R=mark@chromium.org Committed: https://chromium.googlesource.com/breakpad/breakpad/+/11840d0ffd374c2f70fd93686cc9ee63589c738d

Patch Set 1 #

Total comments: 2

Patch Set 2 : More bulletproof version of working with CFReadStreamRead(). #

Total comments: 2

Patch Set 3 : Addressed the concern over minimizing the calls to CFReadStreamRead(). #

Patch Set 4 : Remove whitespace. #

Total comments: 8

Patch Set 5 : Tweaked to conform to stylistic suggestions. #

Patch Set 6 : Fixing a merge error. #

Total comments: 2

Patch Set 7 : Moving three lines inside the loop. #

Patch Set 8 : Merged with latest changes. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -6 lines) Patch
M src/client/mac/handler/minidump_generator.cc View 1 2 3 4 5 6 1 chunk +19 lines, -6 lines 0 comments Download

Messages

Total messages: 22 (3 generated)
ivanpe
4 years, 10 months ago (2016-01-26 02:04:20 UTC) #2
Mark Mentovai
LGTM https://codereview.chromium.org/1637433003/diff/1/src/client/mac/handler/minidump_generator.cc File src/client/mac/handler/minidump_generator.cc (right): https://codereview.chromium.org/1637433003/diff/1/src/client/mac/handler/minidump_generator.cc#newcode152 src/client/mac/handler/minidump_generator.cc:152: if (num_bytes_read > 0) { If num_bytes_read == ...
4 years, 10 months ago (2016-01-26 14:08:51 UTC) #3
ivanpe
https://codereview.chromium.org/1637433003/diff/1/src/client/mac/handler/minidump_generator.cc File src/client/mac/handler/minidump_generator.cc (right): https://codereview.chromium.org/1637433003/diff/1/src/client/mac/handler/minidump_generator.cc#newcode152 src/client/mac/handler/minidump_generator.cc:152: if (num_bytes_read > 0) { On 2016/01/26 14:08:50, Mark ...
4 years, 10 months ago (2016-01-27 18:35:42 UTC) #4
Mark Mentovai
https://codereview.chromium.org/1637433003/diff/20001/src/client/mac/handler/minidump_generator.cc File src/client/mac/handler/minidump_generator.cc (right): https://codereview.chromium.org/1637433003/diff/20001/src/client/mac/handler/minidump_generator.cc#newcode145 src/client/mac/handler/minidump_generator.cc:145: // Actual data file tests: Mac at 480 bytes ...
4 years, 10 months ago (2016-01-27 18:47:09 UTC) #5
Olivier
On 2016/01/27 18:47:09, Mark Mentovai wrote: > https://codereview.chromium.org/1637433003/diff/20001/src/client/mac/handler/minidump_generator.cc > File src/client/mac/handler/minidump_generator.cc (right): > > https://codereview.chromium.org/1637433003/diff/20001/src/client/mac/handler/minidump_generator.cc#newcode145 ...
4 years, 10 months ago (2016-01-28 08:25:49 UTC) #6
ivanpe
Mark, can you please take another look at the latest patch by Scott.
4 years, 10 months ago (2016-01-28 19:29:14 UTC) #8
Mark Mentovai
#3 doesn’t really address my comments to #2
4 years, 10 months ago (2016-01-28 19:31:12 UTC) #9
ivanpe
On 2016/01/28 19:31:12, Mark Mentovai wrote: > #3 doesn’t really address my comments to #2 ...
4 years, 10 months ago (2016-01-28 19:36:37 UTC) #10
seh1
On 2016/01/28 19:36:37, ivanpe wrote: > On 2016/01/28 19:31:12, Mark Mentovai wrote: > > #3 ...
4 years, 10 months ago (2016-01-28 19:39:37 UTC) #11
Mark Mentovai
On 2016/01/28 19:39:37, seh1 wrote: > The 2 problems were not reading all the data ...
4 years, 10 months ago (2016-01-28 19:42:01 UTC) #12
seh1
On 2016/01/28 19:42:01, Mark Mentovai wrote: > On 2016/01/28 19:39:37, seh1 wrote: > > The ...
4 years, 10 months ago (2016-01-29 05:53:12 UTC) #13
Mark Mentovai
https://codereview.chromium.org/1637433003/diff/60001/src/client/mac/handler/minidump_generator.cc File src/client/mac/handler/minidump_generator.cc (right): https://codereview.chromium.org/1637433003/diff/60001/src/client/mac/handler/minidump_generator.cc#newcode156 src/client/mac/handler/minidump_generator.cc:156: if (num_bytes_read < kMaxBufferLength) break; seh wrote: > More ...
4 years, 10 months ago (2016-01-29 14:40:11 UTC) #14
seh1
On 2016/01/29 14:40:11, Mark Mentovai wrote: > https://codereview.chromium.org/1637433003/diff/60001/src/client/mac/handler/minidump_generator.cc > File src/client/mac/handler/minidump_generator.cc (right): > > https://codereview.chromium.org/1637433003/diff/60001/src/client/mac/handler/minidump_generator.cc#newcode156 ...
4 years, 10 months ago (2016-01-30 22:37:19 UTC) #15
Mark Mentovai
> Was simply trying to avoid multiple calls to CFReadStreamRead(), How can you say that ...
4 years, 10 months ago (2016-01-31 02:12:05 UTC) #16
seh1
On 2016/01/31 02:12:05, Mark Mentovai wrote: > > Was simply trying to avoid multiple calls ...
4 years, 10 months ago (2016-01-31 08:21:58 UTC) #17
ivanpe
On 2016/01/31 08:21:58, seh1 wrote: > On 2016/01/31 02:12:05, Mark Mentovai wrote: > > > ...
4 years, 10 months ago (2016-01-31 19:29:10 UTC) #18
Mark Mentovai
LGTM otherwise https://codereview.chromium.org/1637433003/diff/100001/src/client/mac/handler/minidump_generator.cc File src/client/mac/handler/minidump_generator.cc (right): https://codereview.chromium.org/1637433003/diff/100001/src/client/mac/handler/minidump_generator.cc#newcode146 src/client/mac/handler/minidump_generator.cc:146: const CFIndex kMaxBufferLength = 1024; You can ...
4 years, 10 months ago (2016-02-01 01:03:36 UTC) #19
ivanpe
Committing shortly. https://codereview.chromium.org/1637433003/diff/100001/src/client/mac/handler/minidump_generator.cc File src/client/mac/handler/minidump_generator.cc (right): https://codereview.chromium.org/1637433003/diff/100001/src/client/mac/handler/minidump_generator.cc#newcode146 src/client/mac/handler/minidump_generator.cc:146: const CFIndex kMaxBufferLength = 1024; On 2016/02/01 ...
4 years, 10 months ago (2016-02-01 02:05:20 UTC) #20
ivanpe
4 years, 10 months ago (2016-02-01 02:17:47 UTC) #22
Message was sent while issue was closed.
Committed patchset #8 (id:140001) manually as
11840d0ffd374c2f70fd93686cc9ee63589c738d (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698