Remove ScopedVector from chrome/test/chromedriver/

Remove ScopedVector from chrome/test/chromedriver/

base::ScopedVector is deprecated, see bug.

BUG=554289
Review-Url: https://codereview.chromium.org/2777883003
Cr-Commit-Position: refs/heads/master@{#461055}
Committed: https://chromium.googlesource.com/chromium/src/+/c8532b9dd196ec720d9a6b017bca1fb32d7cf226

[leonhsl(Using Gerrit), 2017-03-28 09:05:20]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-28 09:05:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-28 09:19:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-28 09:19:30]

Dry run: This issue passed the CQ dry run.

[leonhsl(Using Gerrit), 2017-03-29 00:43:13]

leon.han@intel.com changed reviewers:
+ avi@chromium.org, samuong@chromium.org

[leonhsl(Using Gerrit), 2017-03-29 00:43:13]

PTAL, Thanks!

[Avi (use Gerrit), 2017-03-29 03:26:55]

As it is, it's not worse than what we have, but I wonder if we can clean it up
as we go. (LGTM)

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
File chrome/test/chromedriver/chrome/chrome_android_impl.cc (right):

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome/chrome_android_impl.cc:20:
devtools_event_listeners,
ditto

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
File chrome/test/chromedriver/chrome/chrome_android_impl.h (right):

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome/chrome_android_impl.h:23:
std::vector<std::unique_ptr<DevToolsEventListener>>&
BTW this is in violation of the style guide (no non-const references), and I
imagine that it was done this way because we didn't have std::move yet.

Can we switch to just a vector<unique_ptr<xxx>> and use std::move?

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
File chrome/test/chromedriver/chrome/chrome_desktop_impl.h (right):

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome/chrome_desktop_impl.h:33:
devtools_event_listeners,
etc

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
File chrome/test/chromedriver/chrome/chrome_impl.cc (right):

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome/chrome_impl.cc:176:
devtools_event_listeners_.swap(devtools_event_listeners);
Yep, we pass it in by value all the way, and then std::move/swap here.

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
File chrome/test/chromedriver/chrome_launcher.cc (right):

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome_launcher.cc:281: devtools_event_listeners,
This is passed by pointer, yes, but down on line 297 is passed into the "new
ChromeRemoteImpl" call which steals it.

If we could also turn this one into a pass-by-value and use moving, that would
be cleaner.

[leonhsl(Using Gerrit), 2017-03-29 09:07:27]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-29 09:07:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-29 09:21:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-29 09:21:44]

Dry run: This issue passed the CQ dry run.

[leonhsl(Using Gerrit), 2017-03-29 09:22:31]

Thanks Avi a lot for the suggestions! Uploaded ps#2, PTAnL, it's really getting
cleaner!

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
File chrome/test/chromedriver/chrome/chrome_android_impl.cc (right):

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome/chrome_android_impl.cc:20:
devtools_event_listeners,
On 2017/03/29 03:26:55, Avi wrote:
> ditto

Done.

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
File chrome/test/chromedriver/chrome/chrome_android_impl.h (right):

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome/chrome_android_impl.h:23:
std::vector<std::unique_ptr<DevToolsEventListener>>&
On 2017/03/29 03:26:55, Avi wrote:
> BTW this is in violation of the style guide (no non-const references), and I
> imagine that it was done this way because we didn't have std::move yet.
> 
> Can we switch to just a vector<unique_ptr<xxx>> and use std::move?

Done.

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
File chrome/test/chromedriver/chrome/chrome_desktop_impl.h (right):

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome/chrome_desktop_impl.h:33:
devtools_event_listeners,
On 2017/03/29 03:26:55, Avi wrote:
> etc

Done.

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
File chrome/test/chromedriver/chrome/chrome_impl.cc (right):

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome/chrome_impl.cc:176:
devtools_event_listeners_.swap(devtools_event_listeners);
On 2017/03/29 03:26:55, Avi wrote:
> Yep, we pass it in by value all the way, and then std::move/swap here.

Done.

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
File chrome/test/chromedriver/chrome_launcher.cc (right):

https://codereview.chromium.org/2777883003/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome_launcher.cc:281: devtools_event_listeners,
On 2017/03/29 03:26:55, Avi wrote:
> This is passed by pointer, yes, but down on line 297 is passed into the "new
> ChromeRemoteImpl" call which steals it.
> 
> If we could also turn this one into a pass-by-value and use moving, that would
> be cleaner.

Done.

[Avi (use Gerrit), 2017-03-29 14:59:53]

Excellent! Still LGTM.

[leonhsl(Using Gerrit), 2017-03-30 23:53:49]

leon.han@intel.com changed reviewers:
+ stgao@chromium.org

[leonhsl(Using Gerrit), 2017-03-30 23:53:50]

+stgao@, PTAL, Thanks!

[stgao, 2017-03-31 05:32:05]

lgtm with a nit

Many thanks for the cleanup!

https://codereview.chromium.org/2777883003/diff/20001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/console_logger_unittest.cc (right):

https://codereview.chromium.org/2777883003/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/console_logger_unittest.cc:99: private:
sorry, it seems the old code was not in good format. Mind one more space at the
beginning here and below?

[leonhsl(Using Gerrit), 2017-03-31 05:56:43]

Thank you for review :) Uploaded ps#3 to address the nit, and send to CQ now.

https://codereview.chromium.org/2777883003/diff/20001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/console_logger_unittest.cc (right):

https://codereview.chromium.org/2777883003/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/console_logger_unittest.cc:99: private:
On 2017/03/31 05:32:05, stgao(ping after 24h) wrote:
> sorry, it seems the old code was not in good format. Mind one more space at
the
> beginning here and below?

Done.

[leonhsl(Using Gerrit), 2017-03-31 05:56:54]

The CQ bit was checked by leon.han@intel.com

[leonhsl(Using Gerrit), 2017-03-31 05:56:54]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org,
stgao@chromium.org
Link to the patchset: https://codereview.chromium.org/2777883003/#ps40001
(title: "address nit from stgao@")

[commit-bot: I haz the power, 2017-03-31 05:57:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-31 06:13:55]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490939814196620,
"parent_rev": "fc68845df1f3a816c0bddcbf4f05f8e45b8e6e4b", "commit_rev":
"c8532b9dd196ec720d9a6b017bca1fb32d7cf226"}

[commit-bot: I haz the power, 2017-03-31 06:15:51]

Description was changed from

==========
Remove ScopedVector from chrome/test/chromedriver/

base::ScopedVector is deprecated, see bug.

BUG=554289
==========

to

==========
Remove ScopedVector from chrome/test/chromedriver/

base::ScopedVector is deprecated, see bug.

BUG=554289

Review-Url: https://codereview.chromium.org/2777883003
Cr-Commit-Position: refs/heads/master@{#461055}
Committed:
https://chromium.googlesource.com/chromium/src/+/c8532b9dd196ec720d9a6b017bca...
==========

[commit-bot: I haz the power, 2017-03-31 06:15:52]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/c8532b9dd196ec720d9a6b017bca...