Moves GPUMetricsProvider to //components/metrics.

Moves GPUMetricsProvider to //components/metrics.

R=asvitkine@chromium.org,isherman@chromium.org
BUG=404791
Committed: https://crrev.com/1afc65175c84ddf6087f65da818bbdbba6d28101
Cr-Commit-Position: refs/heads/master@{#294935}

[Ilya Sherman, 2014-08-30 00:52:03]

LGTM % nit.  Thanks!

https://codereview.chromium.org/523273003/diff/1/components/metrics/gpu/gpu_m...
File components/metrics/gpu/gpu_metrics_provider.h (right):

https://codereview.chromium.org/523273003/diff/1/components/metrics/gpu/gpu_m...
components/metrics/gpu/gpu_metrics_provider.h:20: // MetricsProvider
implementation:
nit: Please omit " implementation" -- this is a common style in Chromium code.

[gunsch, 2014-08-30 15:17:26]

gunsch@chromium.org changed reviewers:
+ piman@chromium.org

[gunsch, 2014-08-30 15:17:27]

+piman for OWNERS (+content/public/browser/gpu_data_manager.h, +gpu/config)

https://codereview.chromium.org/523273003/diff/1/components/metrics/gpu/gpu_m...
File components/metrics/gpu/gpu_metrics_provider.h (right):

https://codereview.chromium.org/523273003/diff/1/components/metrics/gpu/gpu_m...
components/metrics/gpu/gpu_metrics_provider.h:20: // MetricsProvider
implementation:
On 2014/08/30 00:52:03, Ilya Sherman wrote:
> nit: Please omit " implementation" -- this is a common style in Chromium code.

Done---"[class] implementation:" seems a common style as well, was there a
deliberate recent change away from that?

[piman, 2014-09-02 18:52:44]

On 2014/08/30 15:17:27, gunsch_OOO_till_9-8 wrote:
> +piman for OWNERS (+content/public/browser/gpu_data_manager.h, +gpu/config)

I'm not sure what I'm expected to review, those files are not present in the CL.


> 
>
https://codereview.chromium.org/523273003/diff/1/components/metrics/gpu/gpu_m...
> File components/metrics/gpu/gpu_metrics_provider.h (right):
> 
>
https://codereview.chromium.org/523273003/diff/1/components/metrics/gpu/gpu_m...
> components/metrics/gpu/gpu_metrics_provider.h:20: // MetricsProvider
> implementation:
> On 2014/08/30 00:52:03, Ilya Sherman wrote:
> > nit: Please omit " implementation" -- this is a common style in Chromium
code.
> 
> Done---"[class] implementation:" seems a common style as well, was there a
> deliberate recent change away from that?

[Alexei Svitkine (slow), 2014-09-02 19:08:19]

lgtm

[gunsch, 2014-09-08 16:32:56]

On 2014/09/02 18:52:44, piman (OOO) wrote:
> On 2014/08/30 15:17:27, gunsch_OOO_till_9-8 wrote:
> > +piman for OWNERS (+content/public/browser/gpu_data_manager.h, +gpu/config)
> 
> I'm not sure what I'm expected to review, those files are not present in the
CL.
> 
> 
> > 
> >
>
https://codereview.chromium.org/523273003/diff/1/components/metrics/gpu/gpu_m...
> > File components/metrics/gpu/gpu_metrics_provider.h (right):
> > 
> >
>
https://codereview.chromium.org/523273003/diff/1/components/metrics/gpu/gpu_m...
> > components/metrics/gpu/gpu_metrics_provider.h:20: // MetricsProvider
> > implementation:
> > On 2014/08/30 00:52:03, Ilya Sherman wrote:
> > > nit: Please omit " implementation" -- this is a common style in Chromium
> code.
> > 
> > Done---"[class] implementation:" seems a common style as well, was there a
> > deliberate recent change away from that?

piman@: those files are added to a new DEPS file (components/metrics/gpu/DEPS).

[piman, 2014-09-08 18:29:54]

lgtm

[gunsch, 2014-09-08 19:37:51]

The CQ bit was checked by gunsch@chromium.org

[commit-bot: I haz the power, 2014-09-08 19:38:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/gunsch@chromium.org/523273003/20001

[gunsch, 2014-09-08 19:52:24]

The CQ bit was unchecked by gunsch@chromium.org

[gunsch, 2014-09-08 21:56:27]

The CQ bit was checked by gunsch@chromium.org

[commit-bot: I haz the power, 2014-09-08 21:58:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/gunsch@chromium.org/523273003/40001

[gunsch, 2014-09-09 00:02:57]

The CQ bit was unchecked by gunsch@chromium.org

[gunsch, 2014-09-09 00:51:03]

The CQ bit was checked by gunsch@chromium.org

[commit-bot: I haz the power, 2014-09-09 00:52:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/gunsch@chromium.org/523273003/40001

[commit-bot: I haz the power, 2014-09-09 01:25:21]

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

[commit-bot: I haz the power, 2014-09-09 01:25:22]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[gunsch, 2014-09-09 16:08:31]

The CQ bit was checked by gunsch@chromium.org

[gunsch, 2014-09-09 16:08:32]

The CQ bit was unchecked by gunsch@chromium.org

[gunsch, 2014-09-09 16:08:32]

The CQ bit was checked by gunsch@chromium.org

[gunsch, 2014-09-09 16:08:32]

The CQ bit was unchecked by gunsch@chromium.org

[lcwu1, 2014-09-09 16:09:56]

The CQ bit was checked by lcwu@chromium.org

[commit-bot: I haz the power, 2014-09-09 16:10:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/gunsch@chromium.org/523273003/60001

[gunsch, 2014-09-09 16:13:40]

The CQ bit was unchecked by gunsch@chromium.org

[gunsch, 2014-09-09 16:27:33]

The CQ bit was checked by gunsch@chromium.org

[commit-bot: I haz the power, 2014-09-09 16:30:03]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/gunsch@chromium.org/523273003/80001

[gunsch, 2014-09-09 16:57:01]

gunsch@chromium.org changed reviewers:
+ brettw@chromium.org

[gunsch, 2014-09-09 16:57:20]

+brettw for chrome/browser/BUILD.gn

[commit-bot: I haz the power, 2014-09-09 17:14:19]

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

[commit-bot: I haz the power, 2014-09-09 17:14:20]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[brettw, 2014-09-09 17:56:57]

https://codereview.chromium.org/523273003/diff/80001/components/metrics/gpu/DEPS
File components/metrics/gpu/DEPS (right):

https://codereview.chromium.org/523273003/diff/80001/components/metrics/gpu/D...
components/metrics/gpu/DEPS:2: "+content/public/browser/gpu_data_manager.h",
I don't understand this line.

[gunsch, 2014-09-09 18:42:19]

https://codereview.chromium.org/523273003/diff/80001/components/metrics/gpu/DEPS
File components/metrics/gpu/DEPS (right):

https://codereview.chromium.org/523273003/diff/80001/components/metrics/gpu/D...
components/metrics/gpu/DEPS:2: "+content/public/browser/gpu_data_manager.h",
On 2014/09/09 17:56:57, brettw wrote:
> I don't understand this line.

What don't you understand?
GPUMetricsProvider depends on content/public/browser/gpu_data_manager.h.
Instead of adding an include rule for all of content/public/browser, I narrowed
as much as possible.

[brettw, 2014-09-10 00:13:08]

On 2014/09/09 18:42:19, gunsch wrote:
>
https://codereview.chromium.org/523273003/diff/80001/components/metrics/gpu/DEPS
> File components/metrics/gpu/DEPS (right):
> 
>
https://codereview.chromium.org/523273003/diff/80001/components/metrics/gpu/D...
> components/metrics/gpu/DEPS:2: "+content/public/browser/gpu_data_manager.h",
> On 2014/09/09 17:56:57, brettw wrote:
> > I don't understand this line.
> 
> What don't you understand?
> GPUMetricsProvider depends on content/public/browser/gpu_data_manager.h.
> Instead of adding an include rule for all of content/public/browser, I
narrowed
> as much as possible.

I see. That's not really what DEPS is for. It's to keep the code modularized and
catch layering violations. If your component expects to use content/browser, it
should say content/public/browser. Otherwise everybody that adds a header will
have to edit the deps, which isn't the idea.

[gunsch, 2014-09-10 00:45:48]

On 2014/09/10 00:13:08, brettw wrote:
> On 2014/09/09 18:42:19, gunsch wrote:
> >
>
https://codereview.chromium.org/523273003/diff/80001/components/metrics/gpu/DEPS
> > File components/metrics/gpu/DEPS (right):
> > 
> >
>
https://codereview.chromium.org/523273003/diff/80001/components/metrics/gpu/D...
> > components/metrics/gpu/DEPS:2: "+content/public/browser/gpu_data_manager.h",
> > On 2014/09/09 17:56:57, brettw wrote:
> > > I don't understand this line.
> > 
> > What don't you understand?
> > GPUMetricsProvider depends on content/public/browser/gpu_data_manager.h.
> > Instead of adding an include rule for all of content/public/browser, I
> narrowed
> > as much as possible.
> 
> I see. That's not really what DEPS is for. It's to keep the code modularized
and
> catch layering violations. If your component expects to use content/browser,
it
> should say content/public/browser. Otherwise everybody that adds a header will
> have to edit the deps, which isn't the idea.

Got it, changed, PTAL

[brettw, 2014-09-12 22:31:10]

lgtm

https://codereview.chromium.org/523273003/diff/100001/components/metrics/BUIL...
File components/metrics/BUILD.gn (right):

https://codereview.chromium.org/523273003/diff/100001/components/metrics/BUIL...
components/metrics/BUILD.gn:52: static_library("gpu") {
Can you make this a source_set, I've been doing that for new stuff.

[gunsch, 2014-09-12 22:58:04]

https://codereview.chromium.org/523273003/diff/100001/components/metrics/BUIL...
File components/metrics/BUILD.gn (right):

https://codereview.chromium.org/523273003/diff/100001/components/metrics/BUIL...
components/metrics/BUILD.gn:52: static_library("gpu") {
On 2014/09/12 22:31:10, brettw wrote:
> Can you make this a source_set, I've been doing that for new stuff.

Done.

[gunsch, 2014-09-12 22:59:34]

The CQ bit was checked by gunsch@chromium.org

[commit-bot: I haz the power, 2014-09-12 23:00:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/523273003/120001

[commit-bot: I haz the power, 2014-09-12 23:47:54]

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

[commit-bot: I haz the power, 2014-09-12 23:47:55]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[gunsch, 2014-09-13 00:43:17]

gunsch@chromium.org changed reviewers:
+ thakis@chomium.org

[gunsch, 2014-09-13 00:44:45]

On 2014/09/12 23:47:55, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
>   linux_chromium_rel_swarming on tryserver.chromium.linux
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

+thakis: can you review as OWNERS of chrome/browser/BUILD.gn? (one line)

I was under the impression the top-level "per-file BUILD.gn=brettw@chromium.org"
meant all BUILD.gn files. Apparently I was wrong!

[Nico, 2014-09-15 22:31:31]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2014-09-15 22:31:32]

chrome/*.gypi and chrome/*.gn lgtm

[gunsch, 2014-09-15 22:32:07]

The CQ bit was checked by gunsch@chromium.org

[gunsch, 2014-09-15 22:34:22]

The CQ bit was unchecked by gunsch@chromium.org

[gunsch, 2014-09-15 22:38:41]

The CQ bit was checked by gunsch@chromium.org

[commit-bot: I haz the power, 2014-09-15 22:55:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/523273003/140001

[commit-bot: I haz the power, 2014-09-16 00:05:10]

Committed patchset #8 (id:140001) as c1f9b675c7908c57dda6101ad2a7c52cad05d518

[commit-bot: I haz the power, 2014-09-16 00:06:57]

Patchset 8 (id:??) landed as
https://crrev.com/1afc65175c84ddf6087f65da818bbdbba6d28101
Cr-Commit-Position: refs/heads/master@{#294935}