Shore up application testing procedures

Shore up application testing procedures

* Fix TabTest so that it properly resumes and restarts Chrome by
  firing Intents instead of faking it.

* Pull out methods common to ChromeActivityTestCaseBase,
  MultiActivityTestBase, and DocumentModeTestBase into
  a new ApplicationTestUtils class.  Tweaks the methods that are
  very similar but different so that they work the same way.

* Unify the initialization pathways for the two test bases.

BUG=508524, 498973
Committed: https://crrev.com/7ca3b9796a6812c3baa0b7d843d544a5795ff4df
Cr-Commit-Position: refs/heads/master@{#338742}

[gone, 2015-07-10 22:05:49]

dfalcantara@chromium.org changed reviewers:
+ jbudorick@chromium.org

[jbudorick, 2015-07-14 01:26:27]

Generally fine with this. Thanks for doing it.

I launched a bunch of try runs to make sure we don't see any additional
flakiness, particularly with the two TabsTests you're reenabling.

https://codereview.chromium.org/1223323012/diff/20001/chrome/test/android/jav...
File
chrome/test/android/javatests/src/org/chromium/chrome/test/MultiActivityTestBase.java
(right):

https://codereview.chromium.org/1223323012/diff/20001/chrome/test/android/jav...
chrome/test/android/javatests/src/org/chromium/chrome/test/MultiActivityTestBase.java:108:
ApplicationTestUtils.finishAllChromeTasks(mContext);
Not a big fan of the asymmetry of killing tasks in ApplicationTestUtils.setUp
and MultiActivityTestBase.tearDown.

https://codereview.chromium.org/1223323012/diff/20001/chrome/test/android/jav...
File
chrome/test/android/javatests/src/org/chromium/chrome/test/util/ApplicationTestUtils.java
(right):

https://codereview.chromium.org/1223323012/diff/20001/chrome/test/android/jav...
chrome/test/android/javatests/src/org/chromium/chrome/test/util/ApplicationTestUtils.java:26:
public class ApplicationTestUtils {
A lot of these utilities don't appear to be specific to chrome/, so I'm
wondering if we'll eventually want them somewhere else s.t. they can be used by
other tests, e.g. ContentShellTests. (setUp and tearDown are definitely too
specific)

That said, I'm fine with this as-is.

https://codereview.chromium.org/1223323012/diff/20001/chrome/test/android/jav...
chrome/test/android/javatests/src/org/chromium/chrome/test/util/ApplicationTestUtils.java:32:
public static void setUp(Context context, boolean clearAppData, boolean
checkHttpServer)
Not crazy about the flags -- does it make sense to handle these orthogonal setup
pieces in separate functions?

[gone, 2015-07-14 18:55:15]

https://chromiumcodereview.appspot.com/1223323012/diff/20001/chrome/test/andr...
File
chrome/test/android/javatests/src/org/chromium/chrome/test/MultiActivityTestBase.java
(right):

https://chromiumcodereview.appspot.com/1223323012/diff/20001/chrome/test/andr...
chrome/test/android/javatests/src/org/chromium/chrome/test/MultiActivityTestBase.java:108:
ApplicationTestUtils.finishAllChromeTasks(mContext);
On 2015/07/14 01:26:26, jbudorick wrote:
> Not a big fan of the asymmetry of killing tasks in ApplicationTestUtils.setUp
> and MultiActivityTestBase.tearDown.

Shouldn't be problematic; I'll copy it over.

https://chromiumcodereview.appspot.com/1223323012/diff/20001/chrome/test/andr...
File
chrome/test/android/javatests/src/org/chromium/chrome/test/util/ApplicationTestUtils.java
(right):

https://chromiumcodereview.appspot.com/1223323012/diff/20001/chrome/test/andr...
chrome/test/android/javatests/src/org/chromium/chrome/test/util/ApplicationTestUtils.java:26:
public class ApplicationTestUtils {
On 2015/07/14 01:26:27, jbudorick wrote:
> A lot of these utilities don't appear to be specific to chrome/, so I'm
> wondering if we'll eventually want them somewhere else s.t. they can be used
by
> other tests, e.g. ContentShellTests. (setUp and tearDown are definitely too
> specific)
> 
> That said, I'm fine with this as-is.

Yeah, wouldn't hurt.  Can tackle those when the need arises; I've only been
mucking about chrome/ code recently.

https://chromiumcodereview.appspot.com/1223323012/diff/20001/chrome/test/andr...
chrome/test/android/javatests/src/org/chromium/chrome/test/util/ApplicationTestUtils.java:32:
public static void setUp(Context context, boolean clearAppData, boolean
checkHttpServer)
On 2015/07/14 01:26:27, jbudorick wrote:
> Not crazy about the flags -- does it make sense to handle these orthogonal
setup
> pieces in separate functions?

The clearAppData flag isn't set anywhere, and as far as I know it's just a
"convenience" holdover from forever ago.  I've nuked it.

The checkHttpServer flag is only used on one test, but I don't know a clean way
around removing it because it's done in setUp.  Would you be fine with a single
boolean?

[jbudorick, 2015-07-14 21:02:44]

lgtm

https://codereview.chromium.org/1223323012/diff/60001/chrome/test/android/jav...
File
chrome/test/android/javatests/src/org/chromium/chrome/test/util/ApplicationTestUtils.java
(right):

https://codereview.chromium.org/1223323012/diff/60001/chrome/test/android/jav...
chrome/test/android/javatests/src/org/chromium/chrome/test/util/ApplicationTestUtils.java:35:
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP)
finishAllChromeTasks(context);
Is this OK for non-MultiActivityTestCase tests running on L, particularly ones
running against ChromeTabbedActivity?

[gone, 2015-07-14 21:09:00]

https://chromiumcodereview.appspot.com/1223323012/diff/60001/chrome/test/andr...
File
chrome/test/android/javatests/src/org/chromium/chrome/test/util/ApplicationTestUtils.java
(right):

https://chromiumcodereview.appspot.com/1223323012/diff/60001/chrome/test/andr...
chrome/test/android/javatests/src/org/chromium/chrome/test/util/ApplicationTestUtils.java:35:
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP)
finishAllChromeTasks(context);
On 2015/07/14 21:02:44, jbudorick wrote:
> Is this OK for non-MultiActivityTestCase tests running on L, particularly ones
> running against ChromeTabbedActivity?

Should be fine... that particular function just nukes any tasks that
ActivityManager knows about.  ChromeTabbedActivity shouldn't be running at this
point, so the worst thing that can happen is that the ActivityManager just wipes
it from the recent tasks list.

[jbudorick, 2015-07-14 21:09:29]

On 2015/07/14 at 21:09:00, dfalcantara wrote:
>
https://chromiumcodereview.appspot.com/1223323012/diff/60001/chrome/test/andr...
> File
chrome/test/android/javatests/src/org/chromium/chrome/test/util/ApplicationTestUtils.java
(right):
> 
>
https://chromiumcodereview.appspot.com/1223323012/diff/60001/chrome/test/andr...
>
chrome/test/android/javatests/src/org/chromium/chrome/test/util/ApplicationTestUtils.java:35:
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP)
finishAllChromeTasks(context);
> On 2015/07/14 21:02:44, jbudorick wrote:
> > Is this OK for non-MultiActivityTestCase tests running on L, particularly
ones
> > running against ChromeTabbedActivity?
> 
> Should be fine... that particular function just nukes any tasks that
ActivityManager knows about.  ChromeTabbedActivity shouldn't be running at this
point, so the worst thing that can happen is that the ActivityManager just wipes
it from the recent tasks list.

sgtm

[gone, 2015-07-14 21:29:31]

The CQ bit was checked by dfalcantara@chromium.org

[commit-bot: I haz the power, 2015-07-14 21:31:03]

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

[commit-bot: I haz the power, 2015-07-14 22:13:26]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-07-14 22:14:43]

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