Skip to content

Commit

Permalink
Make base::AutoReset moveable
Browse files Browse the repository at this point in the history
This was brought up in the discussion of
https://crrev.com/c/1611240 that we should make it
moveable to avoid heap allocations when we have to
pass it around.

TBR=oshima@chromium.org
BUG=866622
TEST=base_unittests, and existing ash_unittests.

Change-Id: I7c6fb3c010c01129e6dcc8a0155bb53d35d295d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1613371
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#660112}
  • Loading branch information
Ahmed Fakhry authored and Commit Bot committed May 15, 2019
1 parent df499b7 commit 3571b4f
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 7 deletions.
6 changes: 2 additions & 4 deletions ash/wm/desks/desk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,8 @@ void Desk::AddWindowToDesk(aura::Window* window) {
NotifyContentChanged();
}

std::unique_ptr<base::AutoReset<bool>>
Desk::GetScopedNotifyContentChangedDisabler() {
return std::make_unique<base::AutoReset<bool>>(
&should_notify_content_changed_, false);
base::AutoReset<bool> Desk::GetScopedNotifyContentChangedDisabler() {
return base::AutoReset<bool>(&should_notify_content_changed_, false);
}

void Desk::Activate(bool update_window_activation) {
Expand Down
3 changes: 1 addition & 2 deletions ash/wm/desks/desk.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@ class ASH_EXPORT Desk : public aura::WindowObserver {

void AddWindowToDesk(aura::Window* window);

std::unique_ptr<base::AutoReset<bool>>
GetScopedNotifyContentChangedDisabler();
base::AutoReset<bool> GetScopedNotifyContentChangedDisabler();

// Activates this desk. All windows on this desk (if any) will become visible
// (by means of showing this desk's associated containers on all root
Expand Down
1 change: 1 addition & 0 deletions base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2421,6 +2421,7 @@ test("base_unittests") {
"android/unguessable_token_android_unittest.cc",
"at_exit_unittest.cc",
"atomicops_unittest.cc",
"auto_reset_unittests.cc",
"barrier_closure_unittest.cc",
"base64_unittest.cc",
"base64url_unittest.cc",
Expand Down
20 changes: 19 additions & 1 deletion base/auto_reset.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,25 @@ class AutoReset {
*scoped_variable_ = std::move(new_value);
}

~AutoReset() { *scoped_variable_ = std::move(original_value_); }
AutoReset(AutoReset&& other)
: scoped_variable_(other.scoped_variable_),
original_value_(std::move(other.original_value_)) {
other.scoped_variable_ = nullptr;
}

~AutoReset() {
if (scoped_variable_)
*scoped_variable_ = std::move(original_value_);
}

AutoReset& operator=(AutoReset&& rhs) {
if (this != &rhs) {
scoped_variable_ = rhs.scoped_variable_;
rhs.scoped_variable_ = nullptr;
original_value_ = std::move(rhs.original_value_);
}
return *this;
}

private:
T* scoped_variable_;
Expand Down
33 changes: 33 additions & 0 deletions base/auto_reset_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/auto_reset.h"

#include <utility>

#include "testing/gtest/include/gtest/gtest.h"

namespace base {

TEST(AutoReset, Move) {
int value = 10;
{
AutoReset<int> resetter1{&value, 20};
EXPECT_EQ(20, value);
{
value = 15;
AutoReset<int> resetter2 = std::move(resetter1);
// Moving to a new resetter does not change the value;
EXPECT_EQ(15, value);
}
// Moved-to `resetter2` is out of scoped, and resets to the original value
// that was in moved-from `resetter1`.
EXPECT_EQ(10, value);
value = 105;
}
// Moved-from `resetter1` does not reset to anything.
EXPECT_EQ(105, value);
}

} // namespace base

0 comments on commit 3571b4f

Please sign in to comment.