Remove ScopedVector in //component/metrics

Remove ScopedVector in //component/metrics

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

[ke.he, 2017-02-12 07:06:09]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-12 07:06:19]

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-02-12 07:13:27]

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

[commit-bot: I haz the power, 2017-02-12 07:13:28]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[ke.he, 2017-02-12 07:42:40]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-12 07:42:53]

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-02-12 07:50:40]

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

[commit-bot: I haz the power, 2017-02-12 07:50:42]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[ke.he, 2017-02-12 08:51:04]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-12 08:51:09]

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-02-12 08:58:23]

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

[commit-bot: I haz the power, 2017-02-12 08:58:24]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[ke.he, 2017-02-12 09:16:10]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-12 09:16:23]

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-02-12 10:04:38]

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

[commit-bot: I haz the power, 2017-02-12 10:04:38]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-02-12 10:14:38]

ke.he@intel.com changed reviewers:
+ mpearson@chromium.org

[ke.he, 2017-02-12 10:14:39]

Hi, Mark, Could you PTAL on this? Thanks!

https://codereview.chromium.org/2691803002/diff/60001/components/metrics/metr...
File components/metrics/metrics_log.h (right):

https://codereview.chromium.org/2691803002/diff/60001/components/metrics/metr...
components/metrics/metrics_log.h:98: const
std::vector<std::unique_ptr<MetricsProvider>>& metrics_providers,
I just changed the interface definition.
An alternative way is not to change the interface, while we would need to
convert std::vector<std::unique_ptr<T>> into std::vector<T*> everytime before
calling this function(and some other functions in this class).
Any suggestions? Thanks:)

[Mark P, 2017-02-13 05:08:51]

I know I'm an owner here, but most of those files I've never actually
touched.  Can you check the file history of some of those .cc files and
find a more appropriate reviewer?

thanks,
mark


On Sun, Feb 12, 2017 at 2:14 AM, <ke.he@intel.com> wrote:

> Reviewers: Mark P
> CL: https://codereview.chromium.org/2691803002/
>
> Message:
> Hi, Mark, Could you PTAL on this? Thanks!
>
>
> https://codereview.chromium.org/2691803002/diff/60001/
> components/metrics/metrics_log.h
> File components/metrics/metrics_log.h (right):
>
> https://codereview.chromium.org/2691803002/diff/60001/
> components/metrics/metrics_log.h#newcode98
> components/metrics/metrics_log.h:98: const
> std::vector<std::unique_ptr<MetricsProvider>>& metrics_providers,
> I just changed the interface definition.
> An alternative way is not to change the interface, while we would need
> to convert std::vector<std::unique_ptr<T>> into std::vector<T*>
> everytime before calling this function(and some other functions in this
> class).
> Any suggestions? Thanks:)
>
> Description:
> Remove ScopedVector in //component/metrics
>
> BUG=554289
>
> Affected files (+65, -75 lines):
> M chrome/browser/chromeos/external_metrics.cc
> M chromecast/browser/metrics/external_metrics.cc
> M components/metrics/daily_event.h
> M components/metrics/daily_event.cc
> M components/metrics/metrics_log.h
> M components/metrics/metrics_log.cc
> M components/metrics/metrics_log_unittest.cc
> M components/metrics/metrics_service.h
> M components/metrics/metrics_service.cc
> M components/metrics/metrics_service_unittest.cc
> M components/metrics/serialization/serialization_utils.h
> M components/metrics/serialization/serialization_utils.cc
> M components/metrics/serialization/serialization_utils_unittest.cc
> M ios/chrome/today_extension/today_metrics_logger.mm
>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[ke.he, 2017-02-13 05:21:06]

ke.he@intel.com changed reviewers:
+ isherman@chromium.org

[ke.he, 2017-02-13 05:21:07]

Hi, Isherman@, Could you PTAL on this, Thanks!
Mark, Thanks!

[Ilya Sherman, 2017-02-13 19:21:30]

Thanks for the cleanup!

https://codereview.chromium.org/2691803002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/external_metrics.cc (right):

https://codereview.chromium.org/2691803002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/external_metrics.cc:135:
std::vector<std::unique_ptr<metrics::MetricSample>> samples;
Please #include <vector> and <memory> at the top of this file, since neither of
them is #included by the header.  Please go through for all of the files that
you've modified, and double-check whether these includes are needed there as
well.

https://codereview.chromium.org/2691803002/diff/60001/components/metrics/metr...
File components/metrics/metrics_log.h (right):

https://codereview.chromium.org/2691803002/diff/60001/components/metrics/metr...
components/metrics/metrics_log.h:98: const
std::vector<std::unique_ptr<MetricsProvider>>& metrics_providers,
On 2017/02/12 10:14:39, ke.he wrote:
> I just changed the interface definition.
> An alternative way is not to change the interface, while we would need to
> convert std::vector<std::unique_ptr<T>> into std::vector<T*> everytime before
> calling this function(and some other functions in this class).
> Any suggestions? Thanks:)

I think this change is fine -- there's no intent to transfer ownership or
anything like that, so updating the type seems fine.

[ke.he, 2017-02-14 02:42:44]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-14 02:43:45]

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-02-14 04:24:11]

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

[commit-bot: I haz the power, 2017-02-14 04:24:12]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-02-14 04:24:14]

Isherman@, Thanks! CL is updated, PTAL.

https://codereview.chromium.org/2691803002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/external_metrics.cc (right):

https://codereview.chromium.org/2691803002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/external_metrics.cc:135:
std::vector<std::unique_ptr<metrics::MetricSample>> samples;
On 2017/02/13 19:21:30, Ilya Sherman wrote:
> Please #include <vector> and <memory> at the top of this file, since neither
of
> them is #included by the header.  Please go through for all of the files that
> you've modified, and double-check whether these includes are needed there as
> well.

Done.

[Ilya Sherman, 2017-02-14 21:55:01]

LGTM, thanks!

[ke.he, 2017-02-15 02:15:49]

ke.he@intel.com changed reviewers:
+ olivierrobin@chromium.org, xiyuan@chromium.org

[ke.he, 2017-02-15 02:15:49]

Hi, Olivierrobin@, Could you PTAL on
	"ios/chrome/today_extension/today_metrics_logger.mm"

Hi, Xiyuan@, Could you PTAL on "chrome/browser/chromeos/external_metrics.cc"

Thanks all!

[Olivier, 2017-02-15 08:15:11]

today_extension LGTM

[xiyuan, 2017-02-15 15:53:44]

c/b/chromeos/external_metrics.cc lgtm

[ke.he, 2017-02-16 01:40:10]

The CQ bit was checked by ke.he@intel.com

[commit-bot: I haz the power, 2017-02-16 01:41:52]

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-02-16 02:55:22]

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

[commit-bot: I haz the power, 2017-02-16 02:55:23]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[ke.he, 2017-02-16 03:16:16]

The CQ bit was checked by ke.he@intel.com

[commit-bot: I haz the power, 2017-02-16 03:17:36]

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-02-16 03:44:06]

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

[commit-bot: I haz the power, 2017-02-16 03:44:08]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[ke.he, 2017-02-16 03:52:19]

The CQ bit was checked by ke.he@intel.com

[commit-bot: I haz the power, 2017-02-16 03:52:52]

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-02-16 05:05:30]

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

[commit-bot: I haz the power, 2017-02-16 05:05:33]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ke.he, 2017-02-16 05:39:39]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-16 05:40:22]

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-02-16 09:51:37]

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

[commit-bot: I haz the power, 2017-02-16 09:51:38]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-02-16 12:41:14]

The CQ bit was checked by ke.he@intel.com

[ke.he, 2017-02-16 12:41:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org, olivierrobin@chromium.org, xiyuan@chromium.org
Link to the patchset: https://codereview.chromium.org/2691803002/#ps100001
(title: "code rebase")

[commit-bot: I haz the power, 2017-02-16 12:41:34]

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-02-16 12:45:04]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1487248874084160,
"parent_rev": "04ff52bb66284467ccb43d90800013b89ee8db75", "commit_rev":
"a9da80c9e8fed7474456d88744dcb239dcf14198"}

[commit-bot: I haz the power, 2017-02-16 12:45:51]

Description was changed from

==========
Remove ScopedVector in //component/metrics

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

to

==========
Remove ScopedVector in //component/metrics

BUG=554289

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

[commit-bot: I haz the power, 2017-02-16 12:45:55]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/a9da80c9e8fed7474456d88744dc...