Creating mixin-supporting InProcessBrowserTests

Class MixinBasedBrowserTest was added. This class expands its base class, InProcessBrowserTest, by supporting adding mixins. Mixins can be used to add some features not related directly to the testing process in order not to make the test class too complicated and to set up them in a proper time (at the same time when the corresponding set ups for the main test are run).

A mixin should be a class inherited from MixinBasedBrowserTest::Mixin. Test which wants to use some mixin should call AddMixin(mixin_) from its constructor, where mixin_ should be an instance of e.g. MixinYouWantToUse.

Also, now LoginManagerTest is inherited from MixinBasedBrowserTest.

BUG=395653
Committed: https://crrev.com/dbd9c9b2ea5ab7163e42e0fce071805329bb11c5
Cr-Commit-Position: refs/heads/master@{#295954}

[Lisa Ignatyeva, 2014-09-16 12:18:59]

elizavetai@chromium.org changed reviewers:
+ antrim@chromium.org

[Denis Kuznetsov (DE-MUC), 2014-09-16 14:08:33]

antrim@chromium.org changed reviewers:
+ nkostylev@chromium.org

[Denis Kuznetsov (DE-MUC), 2014-09-16 14:08:34]

https://codereview.chromium.org/574703002/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/mixin_in_process_browser_test.cc (right):

https://codereview.chromium.org/574703002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/mixin_in_process_browser_test.cc:58:
mixins_.push_back(mixin);
Maybe add some check that AddMixin can not be called once setUp was started.

https://codereview.chromium.org/574703002/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/mixin_in_process_browser_test.h (right):

https://codereview.chromium.org/574703002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/mixin_in_process_browser_test.h:13: class
MixinInProcessBrowserTest : public InProcessBrowserTest {
I would think on better name - no need to include "InProcess" in name.
MixinBasedBT, or maybe ConfigurableBT would go.

https://codereview.chromium.org/574703002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/mixin_in_process_browser_test.h:31: virtual void
SetUpInProcessBrowserTestFixture() {}
Some comments for these methods (maybe copied from InProcessBT?)

[Lisa Ignatyeva, 2014-09-17 10:39:37]

https://codereview.chromium.org/574703002/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/mixin_in_process_browser_test.cc (right):

https://codereview.chromium.org/574703002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/mixin_in_process_browser_test.cc:58:
mixins_.push_back(mixin);
On 2014/09/16 14:08:34, Denis Kuznetsov wrote:
> Maybe add some check that AddMixin can not be called once setUp was started.

Done.

https://codereview.chromium.org/574703002/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/mixin_in_process_browser_test.h (right):

https://codereview.chromium.org/574703002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/mixin_in_process_browser_test.h:13: class
MixinInProcessBrowserTest : public InProcessBrowserTest {
On 2014/09/16 14:08:34, Denis Kuznetsov wrote:
> I would think on better name - no need to include "InProcess" in name.
> MixinBasedBT, or maybe ConfigurableBT would go.

Done.

https://codereview.chromium.org/574703002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/mixin_in_process_browser_test.h:31: virtual void
SetUpInProcessBrowserTestFixture() {}
On 2014/09/16 14:08:34, Denis Kuznetsov wrote:
> Some comments for these methods (maybe copied from InProcessBT?)

Done.

[Nikita (slow), 2014-09-17 18:14:26]

Liza, which parts do you expect me to review?

I think if you need another reviewer for this CL, Pavel is a better match for
this particular one.

[Denis Kuznetsov (DE-MUC), 2014-09-17 19:25:40]

On 2014/09/17 18:14:26, Nikita Kostylev wrote:
> Liza, which parts do you expect me to review?
> 
> I think if you need another reviewer for this CL, Pavel is a better match for
> this particular one.

Nikita, I've added you, just to confirm naming convention/file location.

[Denis Kuznetsov (DE-MUC), 2014-09-17 19:27:58]

lgtm with nit

https://chromiumcodereview.appspot.com/574703002/diff/100001/chrome/browser/c...
File chrome/browser/chromeos/login/mixin_based_browser_test.cc (right):

https://chromiumcodereview.appspot.com/574703002/diff/100001/chrome/browser/c...
chrome/browser/chromeos/login/mixin_based_browser_test.cc:61: << "You are trying
to add a mixin after setting up has already started.";
Nit : it could be as simple as "Can't add mixin : already set up".

[Nikita (slow), 2014-09-17 20:41:04]

On 2014/09/17 19:25:40, Denis Kuznetsov wrote:
> On 2014/09/17 18:14:26, Nikita Kostylev wrote:
> > Liza, which parts do you expect me to review?
> > 
> > I think if you need another reviewer for this CL, Pavel is a better match
for
> > this particular one.
> 
> Nikita, I've added you, just to confirm naming convention/file location.

It seems that mixin_based_browser_test.* should it chrome/test.

[Denis Kuznetsov (DE-MUC), 2014-09-18 13:54:03]

On 2014/09/17 20:41:04, Nikita Kostylev wrote:
> On 2014/09/17 19:25:40, Denis Kuznetsov wrote:
> > On 2014/09/17 18:14:26, Nikita Kostylev wrote:
> > > Liza, which parts do you expect me to review?
> > > 
> > > I think if you need another reviewer for this CL, Pavel is a better match
> for
> > > this particular one.
> > 
> > Nikita, I've added you, just to confirm naming convention/file location.
> 
> It seems that mixin_based_browser_test.* should it chrome/test.

And Scott, as owner of chrome/test thinks that such class is not general enough
to be put there.

[Lisa Ignatyeva, 2014-09-22 10:47:47]

The CQ bit was checked by elizavetai@chromium.org

[commit-bot: I haz the power, 2014-09-22 10:47:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/574703002/100001

[Nikita (slow), 2014-09-22 10:57:15]

On 2014/09/18 13:54:03, Denis Kuznetsov wrote:
> On 2014/09/17 20:41:04, Nikita Kostylev wrote:
> > On 2014/09/17 19:25:40, Denis Kuznetsov wrote:
> > > On 2014/09/17 18:14:26, Nikita Kostylev wrote:
> > > > Liza, which parts do you expect me to review?
> > > > 
> > > > I think if you need another reviewer for this CL, Pavel is a better
match
> > for
> > > > this particular one.
> > > 
> > > Nikita, I've added you, just to confirm naming convention/file location.
> > 
> > It seems that mixin_based_browser_test.* should it chrome/test.
> 
> And Scott, as owner of chrome/test thinks that such class is not general
enough
> to be put there.

Fine, let's wait till someone else starts using that and then move.

[commit-bot: I haz the power, 2014-09-22 11:05:56]

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

[commit-bot: I haz the power, 2014-09-22 11:05:57]

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

[Lisa Ignatyeva, 2014-09-22 12:10:44]

The CQ bit was checked by elizavetai@chromium.org

[commit-bot: I haz the power, 2014-09-22 12:11:17]

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

[commit-bot: I haz the power, 2014-09-22 12:46:16]

Committed patchset #7 (id:120001) as bab7932a7402d433821baf60d1e7c0c0b6a1e0ed

[commit-bot: I haz the power, 2014-09-22 12:46:55]

Patchset 7 (id:??) landed as
https://crrev.com/dbd9c9b2ea5ab7163e42e0fce071805329bb11c5
Cr-Commit-Position: refs/heads/master@{#295954}