Move toolbar_model_{impl,delegate}.{cc,h} to components/toolbar.

Move toolbar_model_{impl,delegate}.{cc,h} to components/toolbar.

Now that the file have no bad dependencies they can be moved into the
component. Fixes the #include for pref_service and grit generated files,
update the component DEPS.

Remove obsolete forward-declaration and add missing ones.

TBR=sky@chromium.org,mmenke@chromium.org,piman@chromium.org
BUG=582921
Committed: https://crrev.com/3adc092bb1a55a8142de90dda298611d6a4785eb
Cr-Commit-Position: refs/heads/master@{#373526}

[sdefresne, 2016-02-01 17:55:06]

sdefresne@chromium.org changed reviewers:
+ pkasting@chromium.org

[sdefresne, 2016-02-01 17:55:07]

Please take a look.

[sdefresne, 2016-02-01 17:57:03]

Description was changed from

==========
Move toolbar_model_{impl,delegate}.{cc,h} to components/toolbar.

Now that the file have no bad dependencies they can be moved into the
component. Fixes the #include for pref_service and grit generated files,
update the component DEPS.

BUG=582921
==========

to

==========
Move toolbar_model_{impl,delegate}.{cc,h} to components/toolbar.

Now that the file have no bad dependencies they can be moved into the
component. Fixes the #include for pref_service and grit generated files,
update the component DEPS.

Remove obsolete forward-declaration and add missing ones.

BUG=582921
==========

[Peter Kasting, 2016-02-02 23:38:56]

LGTM

https://codereview.chromium.org/1652153003/diff/20001/components/toolbar/tool...
File components/toolbar/toolbar_model_impl.h (right):

https://codereview.chromium.org/1652153003/diff/20001/components/toolbar/tool...
components/toolbar/toolbar_model_impl.h:27: class ToolbarModelImpl : public
ToolbarModel {
Do we still need to separate the abstract base out here?  It seems like now that
these are in the same directory, the only reason for this is for the test code
to be able to create its own implementation.  If it could get by with the "real"
implementation, we could just collapse these classes.

[sdefresne, 2016-02-03 17:44:01]

On 2016/02/02 at 23:38:56, pkasting wrote:
> LGTM
> 
>
https://codereview.chromium.org/1652153003/diff/20001/components/toolbar/tool...
> File components/toolbar/toolbar_model_impl.h (right):
> 
>
https://codereview.chromium.org/1652153003/diff/20001/components/toolbar/tool...
> components/toolbar/toolbar_model_impl.h:27: class ToolbarModelImpl : public
ToolbarModel {
> Do we still need to separate the abstract base out here?  It seems like now
that these are in the same directory, the only reason for this is for the test
code to be able to create its own implementation.  If it could get by with the
"real" implementation, we could just collapse these classes.

I don't think we still need separate classes. I'll merge them in a followup CL.
Thank you for the review.

[blundell, 2016-02-04 07:21:03]

On 2016/02/03 17:44:01, sdefresne wrote:
> On 2016/02/02 at 23:38:56, pkasting wrote:
> > LGTM
> > 
> >
>
https://codereview.chromium.org/1652153003/diff/20001/components/toolbar/tool...
> > File components/toolbar/toolbar_model_impl.h (right):
> > 
> >
>
https://codereview.chromium.org/1652153003/diff/20001/components/toolbar/tool...
> > components/toolbar/toolbar_model_impl.h:27: class ToolbarModelImpl : public
> ToolbarModel {
> > Do we still need to separate the abstract base out here?  It seems like now
> that these are in the same directory, the only reason for this is for the test
> code to be able to create its own implementation.  If it could get by with the
> "real" implementation, we could just collapse these classes.
> 
> I don't think we still need separate classes. I'll merge them in a followup
CL.
> Thank you for the review.

Keep in mind that this division pre-existed the move of toolbar_model.h to
//components, i.e., it presumably was originally created *precisely* so that
TestToolbarModel could exist.

[sdefresne, 2016-02-04 10:35:34]

https://codereview.chromium.org/1652153003/diff/20001/components/toolbar/tool...
File components/toolbar/toolbar_model_impl.h (right):

https://codereview.chromium.org/1652153003/diff/20001/components/toolbar/tool...
components/toolbar/toolbar_model_impl.h:27: class ToolbarModelImpl : public
ToolbarModel {
On 2016/02/02 at 23:38:56, Peter Kasting wrote:
> Do we still need to separate the abstract base out here?  It seems like now
that these are in the same directory, the only reason for this is for the test
code to be able to create its own implementation.  If it could get by with the
"real" implementation, we could just collapse these classes.

=> crbug.com/584195

[sdefresne, 2016-02-04 11:47:34]

sdefresne@chromium.org changed reviewers:
+ mmenke@chromium.org, piman@chromium.org, sky@chromium.org

[sdefresne, 2016-02-04 11:47:34]

Missing LGTM from OWNERS of dependencies added to DEPS:
    '+net',
    '+ui/base',


mmenke: for new DEPS on net/
sky: for new DEPS on ui/

piman: for
    chrome/browser/chromeos/login/DEPS
    chrome/browser/chromeos/login/ui/simple_web_view_dialog.cc

[sdefresne, 2016-02-04 11:48:58]

Description was changed from

==========
Move toolbar_model_{impl,delegate}.{cc,h} to components/toolbar.

Now that the file have no bad dependencies they can be moved into the
component. Fixes the #include for pref_service and grit generated files,
update the component DEPS.

Remove obsolete forward-declaration and add missing ones.

BUG=582921
==========

to

==========
Move toolbar_model_{impl,delegate}.{cc,h} to components/toolbar.

Now that the file have no bad dependencies they can be moved into the
component. Fixes the #include for pref_service and grit generated files,
update the component DEPS.

Remove obsolete forward-declaration and add missing ones.

TBR=sky@chromium.org,mmenke@chromium.org,piman@chromium.org
BUG=582921
==========

[sdefresne, 2016-02-04 11:49:36]

The CQ bit was checked by sdefresne@chromium.org

[sdefresne, 2016-02-04 11:49:36]

TBR-ing for the new DEPS as they are pre-existing via the dependency on
chrome/browser/ui and since components can depends on both net/ and ui/base/

[sdefresne, 2016-02-04 11:49:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/1652153003/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2016-02-04 11:49:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1652153003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1652153003/40001

[commit-bot: I haz the power, 2016-02-04 12:54:26]

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

[commit-bot: I haz the power, 2016-02-04 12:54:27]

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

[sdefresne, 2016-02-04 13:01:25]

The CQ bit was checked by sdefresne@chromium.org

[sdefresne, 2016-02-04 13:01:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/1652153003/#ps60001
(title: "Add static_cast<int> to fix compilation warning on Win")

[commit-bot: I haz the power, 2016-02-04 13:02:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1652153003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1652153003/60001

[commit-bot: I haz the power, 2016-02-04 14:18:46]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-02-04 14:19:55]

Description was changed from

==========
Move toolbar_model_{impl,delegate}.{cc,h} to components/toolbar.

Now that the file have no bad dependencies they can be moved into the
component. Fixes the #include for pref_service and grit generated files,
update the component DEPS.

Remove obsolete forward-declaration and add missing ones.

TBR=sky@chromium.org,mmenke@chromium.org,piman@chromium.org
BUG=582921
==========

to

==========
Move toolbar_model_{impl,delegate}.{cc,h} to components/toolbar.

Now that the file have no bad dependencies they can be moved into the
component. Fixes the #include for pref_service and grit generated files,
update the component DEPS.

Remove obsolete forward-declaration and add missing ones.

TBR=sky@chromium.org,mmenke@chromium.org,piman@chromium.org
BUG=582921

Committed: https://crrev.com/3adc092bb1a55a8142de90dda298611d6a4785eb
Cr-Commit-Position: refs/heads/master@{#373526}
==========

[commit-bot: I haz the power, 2016-02-04 14:19:57]

Patchset 4 (id:??) landed as
https://crrev.com/3adc092bb1a55a8142de90dda298611d6a4785eb
Cr-Commit-Position: refs/heads/master@{#373526}