Instant/LocalNTP tests cleanup

Instant/LocalNTP tests cleanup

local_ntp_browsertest.cc was actually part of the interactive_ui_tests
target, but most tests there don't need the "interactive" part. This CL
moves the one test that *does* need it into a separate _uitest.cc file,
and moves the _browsertest.cc file into the browser_tests target.

To do this, the uitest-specific parts of InstantTestBase were split out
into a new class InstantUITestBase.

BUG=none

Review-Url: https://codereview.chromium.org/2857023003
Cr-Commit-Position: refs/heads/master@{#469293}
Committed: https://chromium.googlesource.com/chromium/src/+/11b6b3790f4138d11f81a3af384d129383ea976a

[Marc Treib, 2017-05-03 11:30:14]

Description was changed from

==========
Instant/LocalNTP tests cleanup

local_ntp_browsertest.cc was actually part of the interactive_ui_tests
target, but most tests there don't need the interactive part. This CL
moves the one test that *does* need it into a separate _uitest.cc file.

To do this, the uitest-specific parts of InstantTestBase were split out
into a new class InstantUITestBase.

BUG=none
==========

to

==========
Instant/LocalNTP tests cleanup

local_ntp_browsertest.cc was actually part of the interactive_ui_tests
target, but most tests there don't need the interactive part. This CL
moves the one test that *does* need it into a separate _uitest.cc file,
and moves all the remaining ones into the browser_tests target.

To do this, the uitest-specific parts of InstantTestBase were split out
into a new class InstantUITestBase.

BUG=none
==========

[Marc Treib, 2017-05-03 11:48:25]

The CQ bit was checked by treib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-03 11:48: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-05-03 13:27:16]

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

[commit-bot: I haz the power, 2017-05-03 13:27:17]

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

[Marc Treib, 2017-05-03 13:55:38]

The CQ bit was checked by treib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-03 13:55:48]

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-05-03 14:59:07]

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

[commit-bot: I haz the power, 2017-05-03 14:59:08]

Dry run: This issue passed the CQ dry run.

[Marc Treib, 2017-05-03 15:38:14]

Description was changed from

==========
Instant/LocalNTP tests cleanup

local_ntp_browsertest.cc was actually part of the interactive_ui_tests
target, but most tests there don't need the interactive part. This CL
moves the one test that *does* need it into a separate _uitest.cc file,
and moves all the remaining ones into the browser_tests target.

To do this, the uitest-specific parts of InstantTestBase were split out
into a new class InstantUITestBase.

BUG=none
==========

to

==========
Instant/LocalNTP tests cleanup

local_ntp_browsertest.cc was actually part of the interactive_ui_tests
target, but most tests there don't need the "interactive" part. This CL
moves the one test that *does* need it into a separate _uitest.cc file,
and moves the _browsertest.cc file into the browser_tests target.

To do this, the uitest-specific parts of InstantTestBase were split out
into a new class InstantUITestBase.

BUG=none
==========

[Marc Treib, 2017-05-03 15:38:45]

treib@chromium.org changed reviewers:
+ bauerb@chromium.org

[Marc Treib, 2017-05-03 15:38:46]

PTAL!
Lots of touched lines, but it's all just moving stuff around.

[Bernhard Bauer, 2017-05-03 16:48:10]

lgtm

https://codereview.chromium.org/2857023003/diff/60001/chrome/browser/ui/searc...
File chrome/browser/ui/search/instant_extended_interactive_uitest.cc (right):

https://codereview.chromium.org/2857023003/diff/60001/chrome/browser/ui/searc...
chrome/browser/ui/search/instant_extended_interactive_uitest.cc:136: public
InstantUITestBase {
I can't say I'm super happy about using multiple inheritance like this here (and
I think neither would your manager 😉), but then again it's already been this
way... Maybe for a followup, could we think about using composition to get this
additional functionality?

[Marc Treib, 2017-05-04 08:25:40]

Thanks!

https://codereview.chromium.org/2857023003/diff/60001/chrome/browser/ui/searc...
File chrome/browser/ui/search/instant_extended_interactive_uitest.cc (right):

https://codereview.chromium.org/2857023003/diff/60001/chrome/browser/ui/searc...
chrome/browser/ui/search/instant_extended_interactive_uitest.cc:136: public
InstantUITestBase {
On 2017/05/03 16:48:10, Bernhard Bauer wrote:
> I can't say I'm super happy about using multiple inheritance like this here
(and
> I think neither would your manager 😉), but then again it's already been this
> way... Maybe for a followup, could we think about using composition to get
this
> additional functionality?

Agreed re multiple inheritance, but one step at a time :)

Assuming Instant stays unlaunched, much of this class can probably just be
deleted. And yes, the rest can then be used via composition, or just global
helper functions, like the Get*FromJS ones.

[Marc Treib, 2017-05-04 08:25:50]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2017-05-04 08:26:06]

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-05-04 08:31:15]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493886350123440,
"parent_rev": "813e697704faf15bfbc14d8381d8f06eda85c86a", "commit_rev":
"11b6b3790f4138d11f81a3af384d129383ea976a"}

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

Description was changed from

==========
Instant/LocalNTP tests cleanup

local_ntp_browsertest.cc was actually part of the interactive_ui_tests
target, but most tests there don't need the "interactive" part. This CL
moves the one test that *does* need it into a separate _uitest.cc file,
and moves the _browsertest.cc file into the browser_tests target.

To do this, the uitest-specific parts of InstantTestBase were split out
into a new class InstantUITestBase.

BUG=none
==========

to

==========
Instant/LocalNTP tests cleanup

local_ntp_browsertest.cc was actually part of the interactive_ui_tests
target, but most tests there don't need the "interactive" part. This CL
moves the one test that *does* need it into a separate _uitest.cc file,
and moves the _browsertest.cc file into the browser_tests target.

To do this, the uitest-specific parts of InstantTestBase were split out
into a new class InstantUITestBase.

BUG=none

Review-Url: https://codereview.chromium.org/2857023003
Cr-Commit-Position: refs/heads/master@{#469293}
Committed:
https://chromium.googlesource.com/chromium/src/+/11b6b3790f4138d11f81a3af384d...
==========

[commit-bot: I haz the power, 2017-05-04 08:31:32]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/11b6b3790f4138d11f81a3af384d...