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

Issue 2680403004: Update SupportsUserData API to accept std::unique_ptr<>. (Closed)

Created:
3 years, 1 month ago by sdefresne
Modified:
3 years, 1 month ago
Reviewers:
Nico
CC:
chromium-reviews, vmpstr+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Update SupportsUserData API to accept std::unique_ptr<>. SupportsUserData takes ownership of the SupportsUserData::Data passed to SetUserData. Add an overload taking a std::unique_ptr so that client code can be converted incrementally. BUG=690937 Review-Url: https://codereview.chromium.org/2680403004 Cr-Commit-Position: refs/heads/master@{#449933} Committed: https://chromium.googlesource.com/chromium/src/+/ae5e2d28e5254462e5e5d973b788a574b3053b8f

Patch Set 1 #

Total comments: 1

Patch Set 2 : Add TODO. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -3 lines) Patch
M base/supports_user_data.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
M base/supports_user_data.cc View 1 chunk +6 lines, -1 line 0 comments Download
M base/supports_user_data_unittest.cc View 2 chunks +3 lines, -2 lines 0 comments Download

Messages

Total messages: 16 (10 generated)
sdefresne
Please take a look. I know that the style guide recommends against using overloads, but ...
3 years, 1 month ago (2017-02-10 10:52:23 UTC) #2
Nico
if you're planning to do some big change incrementally, please have a tracking bug for ...
3 years, 1 month ago (2017-02-10 15:04:23 UTC) #5
sdefresne
Done => https://bugs.chromium.org/p/chromium/issues/detail?id=690937.
3 years, 1 month ago (2017-02-10 15:07:57 UTC) #7
Nico
lgtm, thanks https://codereview.chromium.org/2680403004/diff/1/base/supports_user_data.h File base/supports_user_data.h (right): https://codereview.chromium.org/2680403004/diff/1/base/supports_user_data.h#newcode41 base/supports_user_data.h:41: void SetUserData(const void* key, Data* data); add ...
3 years, 1 month ago (2017-02-10 17:54:25 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2680403004/20001
3 years, 1 month ago (2017-02-13 10:09:28 UTC) #13
commit-bot: I haz the power
3 years, 1 month ago (2017-02-13 11:50:08 UTC) #16
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/ae5e2d28e5254462e5e5d973b788...

Powered by Google App Engine
This is Rietveld 408576698