Commit 536f7521 authored by Sorin Jianu's avatar Sorin Jianu Committed by Chromium LUCI CQ

Fix non-compliant rvalue ref code in mac/update_service_proxy_test.mm.

This CL fixes two minor coding style issues.

https://google.github.io/styleguide/cppguide.html#Rvalue_references

According to the coding style, rvalue refs can only be used in a
couple of scenarios, and they do not include the following:

1. base::OnceClosure&& done_cb
The idiomatic way is to declare base::OnceClosure function parameters
by value.

2. StateChangeTestEngine::StateChangeTestEngine(
      std::vector<StatePair>&& state_vec)
    : state_seq_(state_vec) {}
This ctor is not one of the cases where overloads on rvalue ref are
allowed.

Also, the code on trunk has a minor bug: because move semantics are not
passed through, std::move needs to be called even though `state_vec`
is a rvalue reference. Otherwise, the member initialization is still
going to be a copy instead of a move.


Bug: 1164718
Change-Id: I3d63cb6bb837121e640381ce1ff0ae96ce08d161
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2619964Reviewed-by: default avatarJoshua Pawlicki <waffles@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843696}
parent d5f01530
...@@ -55,7 +55,7 @@ class StateChangeTestEngine { ...@@ -55,7 +55,7 @@ class StateChangeTestEngine {
// Construct a StateChangeTestEngine that will issue the given states in // Construct a StateChangeTestEngine that will issue the given states in
// the given sequence, and verify that it observes the expected items // the given sequence, and verify that it observes the expected items
// in its callback. // in its callback.
StateChangeTestEngine(std::vector<StatePair>&& state_vec); explicit StateChangeTestEngine(std::vector<StatePair> state_vec);
~StateChangeTestEngine(); ~StateChangeTestEngine();
...@@ -73,7 +73,7 @@ class StateChangeTestEngine { ...@@ -73,7 +73,7 @@ class StateChangeTestEngine {
// of the test - this is intended to be used to send the final reply. // of the test - this is intended to be used to send the final reply.
void StartSimulating( void StartSimulating(
base::scoped_nsprotocol<id<CRUUpdateStateObserving>> observer, base::scoped_nsprotocol<id<CRUUpdateStateObserving>> observer,
base::OnceClosure&& done_cb); base::OnceClosure done_cb);
private: private:
using vec_size_t = std::vector<StatePair>::size_type; using vec_size_t = std::vector<StatePair>::size_type;
...@@ -123,8 +123,8 @@ class StateChangeTestEngine { ...@@ -123,8 +123,8 @@ class StateChangeTestEngine {
const StateChangeTestEngine::vec_size_t StateChangeTestEngine::kNotStarted; const StateChangeTestEngine::vec_size_t StateChangeTestEngine::kNotStarted;
const StateChangeTestEngine::vec_size_t StateChangeTestEngine::kDone; const StateChangeTestEngine::vec_size_t StateChangeTestEngine::kDone;
StateChangeTestEngine::StateChangeTestEngine(std::vector<StatePair>&& state_vec) StateChangeTestEngine::StateChangeTestEngine(std::vector<StatePair> state_vec)
: state_seq_(state_vec) {} : state_seq_(std::move(state_vec)) {}
StateChangeTestEngine::~StateChangeTestEngine() { StateChangeTestEngine::~StateChangeTestEngine() {
EXPECT_NE(next_observation_, kNotStarted) EXPECT_NE(next_observation_, kNotStarted)
...@@ -138,7 +138,7 @@ StateChangeTestEngine::~StateChangeTestEngine() { ...@@ -138,7 +138,7 @@ StateChangeTestEngine::~StateChangeTestEngine() {
void StateChangeTestEngine::StartSimulating( void StateChangeTestEngine::StartSimulating(
base::scoped_nsprotocol<id<CRUUpdateStateObserving>> observer, base::scoped_nsprotocol<id<CRUUpdateStateObserving>> observer,
base::OnceClosure&& done_cb) { base::OnceClosure done_cb) {
EXPECT_TRUE(callback_prepared_) EXPECT_TRUE(callback_prepared_)
<< "TEST ISSUE: StateChangetestEngine cannot StartSimulating without " << "TEST ISSUE: StateChangetestEngine cannot StartSimulating without "
"Watch()ing for event callbacks"; "Watch()ing for event callbacks";
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment