[Part 6 of 6] Move browsertests and utils to task_management/ directory

[Part 6 of 6] Move browsertests and utils to task_management/ directory

Move task_manager_browsertest.cc and
task_manager_browsertest_util.{cc,h} to task_management directory.

[splitting apart task_manager_browsertest_util, part 6 of 6]

BUG=606963
Committed: https://crrev.com/50b06493bfa168ad10da21ca1067dcf1da5945b7
Cr-Commit-Position: refs/heads/master@{#392773}

[ncarter (slow), 2016-05-06 22:32:54]

nick@chromium.org changed reviewers:
+ afakhry@chromium.org

[ncarter (slow), 2016-05-06 22:32:55]

Ahmed, this is the last one, that actually does the move.

[afakhry, 2016-05-07 00:31:51]

https://codereview.chromium.org/1953383002/diff/20001/chrome/browser/task_man...
File chrome/browser/task_management/DEPS (right):

https://codereview.chromium.org/1953383002/diff/20001/chrome/browser/task_man...
chrome/browser/task_management/DEPS:2:
"+extensions/strings/grit/extensions_strings.h"
Why?

https://codereview.chromium.org/1953383002/diff/20001/chrome/browser/task_man...
File chrome/browser/task_management/task_manager_tester.h (right):

https://codereview.chromium.org/1953383002/diff/20001/chrome/browser/task_man...
chrome/browser/task_management/task_manager_tester.h:19: class TaskManagerTester
{
Should that also be under browsertest_utils?

[ncarter (slow), 2016-05-07 22:17:21]

https://codereview.chromium.org/1953383002/diff/20001/chrome/browser/task_man...
File chrome/browser/task_management/DEPS (right):

https://codereview.chromium.org/1953383002/diff/20001/chrome/browser/task_man...
chrome/browser/task_management/DEPS:2:
"+extensions/strings/grit/extensions_strings.h"
On 2016/05/07 00:31:51, afakhry wrote:
> Why?

IDS_EXTENSION_TASK_MANAGER_WEBVIEW_TAG_PREFIX

https://codereview.chromium.org/1953383002/diff/20001/chrome/browser/task_man...
File chrome/browser/task_management/task_manager_tester.h (right):

https://codereview.chromium.org/1953383002/diff/20001/chrome/browser/task_man...
chrome/browser/task_management/task_manager_tester.h:19: class TaskManagerTester
{
On 2016/05/07 00:31:51, afakhry wrote:
> Should that also be under browsertest_utils?

No. The browsertest_utils namespace is just that file.

[afakhry, 2016-05-09 17:57:42]

https://codereview.chromium.org/1953383002/diff/20001/chrome/browser/task_man...
File chrome/browser/task_management/DEPS (right):

https://codereview.chromium.org/1953383002/diff/20001/chrome/browser/task_man...
chrome/browser/task_management/DEPS:2:
"+extensions/strings/grit/extensions_strings.h"
On 2016/05/07 22:17:21, ncarter wrote:
> On 2016/05/07 00:31:51, afakhry wrote:
> > Why?
> 
> IDS_EXTENSION_TASK_MANAGER_WEBVIEW_TAG_PREFIX

That's weird. I'm not seeing the ID:
IDS_EXTENSION_TASK_MANAGER_WEBVIEW_TAG_PREFIX being used anywhere in
c/b/task_management, while I can see it being used in the old c/b/task_manager.
What am I missing?

oh, I guess now we'll be using it now in task_management/ after the move. But
GuestTask is different in that it gets the prefix from the GuestViewBase itself.

But anyways, aren't we allowed to include extensions_strings.h here by default?
Why do we need DEPS? I thought DEPS are used to limit the includes to certain
components.

https://codereview.chromium.org/1953383002/diff/20001/chrome/browser/task_man...
File chrome/browser/task_management/task_manager_tester.h (right):

https://codereview.chromium.org/1953383002/diff/20001/chrome/browser/task_man...
chrome/browser/task_management/task_manager_tester.h:19: class TaskManagerTester
{
On 2016/05/07 22:17:21, ncarter wrote:
> On 2016/05/07 00:31:51, afakhry wrote:
> > Should that also be under browsertest_utils?
> 
> No. The browsertest_utils namespace is just that file.

Sounds good.

[ncarter (slow), 2016-05-09 19:19:55]

https://codereview.chromium.org/1953383002/diff/20001/chrome/browser/task_man...
File chrome/browser/task_management/DEPS (right):

https://codereview.chromium.org/1953383002/diff/20001/chrome/browser/task_man...
chrome/browser/task_management/DEPS:2:
"+extensions/strings/grit/extensions_strings.h"
On 2016/05/09 17:57:41, afakhry wrote:
> On 2016/05/07 22:17:21, ncarter wrote:
> > On 2016/05/07 00:31:51, afakhry wrote:
> > > Why?
> > 
> > IDS_EXTENSION_TASK_MANAGER_WEBVIEW_TAG_PREFIX
> 
> That's weird. I'm not seeing the ID:
> IDS_EXTENSION_TASK_MANAGER_WEBVIEW_TAG_PREFIX being used anywhere in
> c/b/task_management, while I can see it being used in the old
c/b/task_manager.
> What am I missing?
> 
> oh, I guess now we'll be using it now in task_management/ after the move. But
> GuestTask is different in that it gets the prefix from the GuestViewBase
itself.
> 
> But anyways, aren't we allowed to include extensions_strings.h here by
default?
> Why do we need DEPS? I thought DEPS are used to limit the includes to certain
> components.

Right, task_management gets IDS_EXTENSION_TASK_MANAGER_WEBVIEW_TAG_PREFIX via
GuestViewBase::GetTaskPrefix(). However, since that's not a static method, we
can't easily call from MatchWebview(), without an instance.

DEPS is used to enforce layering generally, including but not limited to
components. "extensions/strings" is a component, and #including it here is a
checkdeps error without this change, because there's no higher-level rule
allowing it (i.e. there's no + rule in chrome/DEPS or chrome/browser/DEPS).

[afakhry, 2016-05-09 20:32:21]

lgtm

https://codereview.chromium.org/1953383002/diff/40001/chrome/browser/task_man...
File chrome/browser/task_management/task_manager_tester.h (right):

https://codereview.chromium.org/1953383002/diff/40001/chrome/browser/task_man...
chrome/browser/task_management/task_manager_tester.h:8: #include <stddef.h>
Do we need this here?

[ncarter (slow), 2016-05-09 20:58:50]

https://codereview.chromium.org/1953383002/diff/40001/chrome/browser/task_man...
File chrome/browser/task_management/task_manager_tester.h (right):

https://codereview.chromium.org/1953383002/diff/40001/chrome/browser/task_man...
chrome/browser/task_management/task_manager_tester.h:8: #include <stddef.h>
On 2016/05/09 20:32:21, afakhry wrote:
> Do we need this here?

Yes, it's where int32_t comes from.

[ncarter (slow), 2016-05-09 21:01:51]

https://codereview.chromium.org/1953383002/diff/40001/chrome/browser/task_man...
File chrome/browser/task_management/task_manager_tester.h (right):

https://codereview.chromium.org/1953383002/diff/40001/chrome/browser/task_man...
chrome/browser/task_management/task_manager_tester.h:8: #include <stddef.h>
On 2016/05/09 20:58:50, ncarter wrote:
> On 2016/05/09 20:32:21, afakhry wrote:
> > Do we need this here?
> 
> Yes, it's where int32_t comes from.

Oh whoops no. It should be #include <stdint.h>.

[ncarter (slow), 2016-05-10 18:37:42]

nick@chromium.org changed reviewers:
+ thestig@chromium.org

[ncarter (slow), 2016-05-10 18:37:43]

thestig@chromium.org: Please review changes in chrome/browser. This is a
straight move of code. Parts 1-5 have landed already, and contained the
functional changes.

[Lei Zhang, 2016-05-10 18:43:22]

Description was changed from

==========
[Part 6 of 6] Move browsertests and utils to task_management/ directory

Move task_manager_browsertest.cc and task_manager_browsertest_util.{cc,h} to
task_management directory.

[splitting apart task_manager_browsertest_util, part 6 of 6]

BUG=606963
==========

to

==========
[Part 6 of 6] Move browsertests and utils to task_management/ directory

Move task_manager_browsertest.cc and
task_manager_browsertest_util.{cc,h} to task_management directory.

[splitting apart task_manager_browsertest_util, part 6 of 6]

BUG=606963
==========

[Lei Zhang, 2016-05-10 18:47:41]

lgtm

I formatted the CL description for you once again. 72 columns.

Parts 1 - 5 already landed -> that was fast.

[ncarter (slow), 2016-05-10 19:36:03]

The CQ bit was checked by nick@chromium.org

[ncarter (slow), 2016-05-10 19:36:04]

The patchset sent to the CQ was uploaded after l-g-t-m from afakhry@chromium.org
Link to the patchset: https://codereview.chromium.org/1953383002/#ps80001
(title: "stddef/stdint")

[commit-bot: I haz the power, 2016-05-10 19:36:48]

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

[commit-bot: I haz the power, 2016-05-10 19:49:54]

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

[commit-bot: I haz the power, 2016-05-10 19:49:55]

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

[ncarter (slow), 2016-05-10 19:57:45]

nick@chromium.org changed reviewers:
+ reillyg@chromium.org

[ncarter (slow), 2016-05-10 19:57:46]

+reillyg to review chrome/browser/task_management/DEPS, which is adding an
include rule for extension_strings.h, which needs it for the
IDS_EXTENSION_TASK_MANAGER_WEBVIEW_TAG_PREFIX symbol in
task_manager_browsertest_util.h. Because I'm moving this file to a different
directory, we need a DEPS rule in chrome/browser/task_management (the new task
manager) in addition to the existing on in the chrome/browser/task_manager
(legacy task manager impl) directory

[Reilly Grant (use Gerrit), 2016-05-10 20:55:31]

lgtm

[ncarter (slow), 2016-05-10 21:03:47]

The CQ bit was checked by nick@chromium.org

[commit-bot: I haz the power, 2016-05-10 21:04:18]

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

[commit-bot: I haz the power, 2016-05-10 23:40:41]

Description was changed from

==========
[Part 6 of 6] Move browsertests and utils to task_management/ directory

Move task_manager_browsertest.cc and
task_manager_browsertest_util.{cc,h} to task_management directory.

[splitting apart task_manager_browsertest_util, part 6 of 6]

BUG=606963
==========

to

==========
[Part 6 of 6] Move browsertests and utils to task_management/ directory

Move task_manager_browsertest.cc and
task_manager_browsertest_util.{cc,h} to task_management directory.

[splitting apart task_manager_browsertest_util, part 6 of 6]

BUG=606963
==========

[commit-bot: I haz the power, 2016-05-10 23:40:47]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-10 23:42:24]

Description was changed from

==========
[Part 6 of 6] Move browsertests and utils to task_management/ directory

Move task_manager_browsertest.cc and
task_manager_browsertest_util.{cc,h} to task_management directory.

[splitting apart task_manager_browsertest_util, part 6 of 6]

BUG=606963
==========

to

==========
[Part 6 of 6] Move browsertests and utils to task_management/ directory

Move task_manager_browsertest.cc and
task_manager_browsertest_util.{cc,h} to task_management directory.

[splitting apart task_manager_browsertest_util, part 6 of 6]

BUG=606963

Committed: https://crrev.com/50b06493bfa168ad10da21ca1067dcf1da5945b7
Cr-Commit-Position: refs/heads/master@{#392773}
==========

[commit-bot: I haz the power, 2016-05-10 23:42:25]

Patchset 5 (id:??) landed as
https://crrev.com/50b06493bfa168ad10da21ca1067dcf1da5945b7
Cr-Commit-Position: refs/heads/master@{#392773}