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

Issue 2681283002: Add ability to return moved value from FunctorMessageHandler, Optional. (Closed)

Created:
3 years, 10 months ago by Taylor Brandstetter
Modified:
3 years, 10 months ago
Reviewers:
tommi, pthatcher1
CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Add ability to return moved value from FunctorMessageHandler, Optional. This functionality is desired for this CL: https://codereview.webrtc.org/2675173003/ BUG=None Review-Url: https://codereview.webrtc.org/2681283002 Cr-Commit-Position: refs/heads/master@{#16546} Committed: https://chromium.googlesource.com/external/webrtc/+/81baed36bff0d331c71d2358bfb4e28dd2eb64ab

Patch Set 1 #

Total comments: 4

Patch Set 2 : Rename to MoveValue as suggested. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -15 lines) Patch
M webrtc/base/messagehandler.h View 1 2 chunks +5 lines, -14 lines 0 comments Download
M webrtc/base/optional.h View 1 1 chunk +6 lines, -0 lines 0 comments Download
M webrtc/base/optional_unittest.cc View 1 1 chunk +15 lines, -0 lines 0 comments Download
M webrtc/base/thread.h View 1 1 chunk +1 line, -1 line 0 comments Download

Dependent Patchsets:

Messages

Total messages: 12 (6 generated)
pthatcher1
lgtm, but I'd like it if other owners of base/ look at it too since ...
3 years, 10 months ago (2017-02-10 18:18:01 UTC) #2
Taylor Brandstetter
PTAL tommi. The only visible effect of this will be that "Invoke" will use a ...
3 years, 10 months ago (2017-02-10 19:29:42 UTC) #4
tommi
lgtm with the preference of naming the method MoveResult since I think that move semantics ...
3 years, 10 months ago (2017-02-10 19:38:55 UTC) #5
Taylor Brandstetter
https://codereview.webrtc.org/2681283002/diff/1/webrtc/base/messagehandler.h File webrtc/base/messagehandler.h (right): https://codereview.webrtc.org/2681283002/diff/1/webrtc/base/messagehandler.h#newcode50 webrtc/base/messagehandler.h:50: ReturnT ConsumeResult() { return std::move(result_); } On 2017/02/10 19:38:55, ...
3 years, 10 months ago (2017-02-11 01:03:30 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/2681283002/20001
3 years, 10 months ago (2017-02-11 01:03:46 UTC) #9
commit-bot: I haz the power
3 years, 10 months ago (2017-02-11 02:11:16 UTC) #12
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/webrtc/+/81baed36bff0d331c71d2358b...

Powered by Google App Engine
This is Rietveld 408576698