Add integration tests for WebAPKs

Add integration tests for WebAPKs

This CL adds an integration test to verify that launching a WebAPK:
- Does not crash
- The initial URL eventually finishes loading
- The splash screen eventually hides

BUG=691848
Review-Url: https://codereview.chromium.org/2701333002
Cr-Commit-Position: refs/heads/master@{#453700}
Committed: https://chromium.googlesource.com/chromium/src/+/6a6e60653e99e4f6ae358a282e2893d05ef04427

[pkotwicz, 2017-02-20 03:47:56]

Patchset #2 (id:20001) has been deleted

[pkotwicz, 2017-02-20 03:48:07]

Patchset #1 (id:1) has been deleted

[pkotwicz, 2017-02-20 03:51:40]

Patchset #1 (id:40001) has been deleted

[pkotwicz, 2017-02-20 03:53:43]

Description was changed from

==========
Add integration tests for WebAPKs

BUG=691848
==========

to

==========
Add integration tests for WebAPKs

This CL adds an integration test to verify that launching a WebAPK:
- Does not crash
- The initial URL eventually finishes loading
- The splash screen eventually hides

BUG=691848
==========

[pkotwicz, 2017-02-20 03:53:44]

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

[pkotwicz, 2017-02-20 03:53:54]

Dan can you please take a look?

[gone, 2017-02-21 22:31:50]

https://codereview.chromium.org/2701333002/diff/60001/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/2701333002/diff/60001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:778:
"//chrome/android/webapk/shell_apk:webapk_for_test",
That's a really weird target name.  It should be webapk_javatests or something
like the other ones.

https://codereview.chromium.org/2701333002/diff/60001/chrome/android/java/And...
File chrome/android/java/AndroidManifest.xml (right):

https://codereview.chromium.org/2701333002/diff/60001/chrome/android/java/And...
chrome/android/java/AndroidManifest.xml:942: <activity
android:name="org.chromium.chrome.browser.webapps.TestWebApkActivity"
Eh, I am strongly against adding a test-specific Activity to the real manifest
-- we'll never be able to remove it.  Will it work if it's in
chrome/android/javatests/AndroidManifest.xml instead?

https://codereview.chromium.org/2701333002/diff/60001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TestWebApkActivity.java
(right):

https://codereview.chromium.org/2701333002/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TestWebApkActivity.java:8:
public class TestWebApkActivity extends WebApkActivity {
This should really be in the testing packages, not the real ones.

[pkotwicz, 2017-02-24 23:45:41]

Patchset #3 (id:100001) has been deleted

[pkotwicz, 2017-02-24 23:49:55]

Dan, can you please take another look?

https://codereview.chromium.org/2701333002/diff/60001/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/2701333002/diff/60001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:778:
"//chrome/android/webapk/shell_apk:webapk_for_test",
On 2017/02/21 22:31:50, dfalcantara (load balance plz) wrote:
> That's a really weird target name.  It should be webapk_javatests or something
> like the other ones.

Done.

https://codereview.chromium.org/2701333002/diff/60001/chrome/android/java/And...
File chrome/android/java/AndroidManifest.xml (right):

https://codereview.chromium.org/2701333002/diff/60001/chrome/android/java/And...
chrome/android/java/AndroidManifest.xml:942: <activity
android:name="org.chromium.chrome.browser.webapps.TestWebApkActivity"
That's what I meant to do. Thank you very much for catching!

I ended up removing TestWebApkActivity. Instrumentation#startActivitySync() does
not like activities defined in the test application (instead of defined in the
application under test) I have now added a static method
WebApkUpdateManager#setUpdatesEnabledForTesting()

https://codereview.chromium.org/2701333002/diff/60001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TestWebApkActivity.java
(right):

https://codereview.chromium.org/2701333002/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TestWebApkActivity.java:8:
public class TestWebApkActivity extends WebApkActivity {
On 2017/02/21 22:31:50, dfalcantara (load balance plz) wrote:
> This should really be in the testing packages, not the real ones.

Done.

[gone, 2017-02-27 22:31:08]

lgtm, but you're going to need someone to review the GN changes; I'm not
familiar enough with it :/

https://codereview.chromium.org/2701333002/diff/120001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkIntegrationTest.java
(right):

https://codereview.chromium.org/2701333002/diff/120001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkIntegrationTest.java:82:
WebApkUpdateManager.setUpdatesEnabledForTesting(false);
Should this be in setUp(), above the call to super.setUp()?  Not sure what other
tests you're planning to add.

[pkotwicz, 2017-02-28 01:56:36]

https://codereview.chromium.org/2701333002/diff/120001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkIntegrationTest.java
(right):

https://codereview.chromium.org/2701333002/diff/120001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkIntegrationTest.java:82:
WebApkUpdateManager.setUpdatesEnabledForTesting(false);
Done! I wasn't planning on adding any other tests actually

[pkotwicz, 2017-02-28 01:56:46]

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

[commit-bot: I haz the power, 2017-02-28 01:57:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pkotwicz, 2017-02-28 01:58:47]

pkotwicz@chromium.org changed reviewers:
+ agrieve@chromium.org

[pkotwicz, 2017-02-28 01:58:48]

Adding agrieve@ to review the GN file changes ⛷

[commit-bot: I haz the power, 2017-02-28 03:09:49]

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

[commit-bot: I haz the power, 2017-02-28 03:09:50]

Dry run: This issue passed the CQ dry run.

[agrieve, 2017-02-28 03:39:10]

just one nit. lgtm

https://codereview.chromium.org/2701333002/diff/140001/chrome/android/webapk/...
File chrome/android/webapk/shell_apk/BUILD.gn (right):

https://codereview.chromium.org/2701333002/diff/140001/chrome/android/webapk/...
chrome/android/webapk/shell_apk/BUILD.gn:140: native_lib_placeholders = [
"libfoo.so" ]
nit: add a comment for this line (guessing it's to force 32 bit mode?)

[pkotwicz, 2017-02-28 19:24:43]

Patchset #5 (id:160001) has been deleted

[pkotwicz, 2017-02-28 19:24:56]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2017-02-28 19:24:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org, agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/2701333002/#ps180001
(title: "Merge branch 'master' into integration_test")

[commit-bot: I haz the power, 2017-02-28 19:25:47]

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-02-28 21:07:14]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1488309896686690,
"parent_rev": "bf2aca8990c89a62651f80ab02dc177a58d83784", "commit_rev":
"6a6e60653e99e4f6ae358a282e2893d05ef04427"}

[commit-bot: I haz the power, 2017-02-28 21:07:54]

Description was changed from

==========
Add integration tests for WebAPKs

This CL adds an integration test to verify that launching a WebAPK:
- Does not crash
- The initial URL eventually finishes loading
- The splash screen eventually hides

BUG=691848
==========

to

==========
Add integration tests for WebAPKs

This CL adds an integration test to verify that launching a WebAPK:
- Does not crash
- The initial URL eventually finishes loading
- The splash screen eventually hides

BUG=691848

Review-Url: https://codereview.chromium.org/2701333002
Cr-Commit-Position: refs/heads/master@{#453700}
Committed:
https://chromium.googlesource.com/chromium/src/+/6a6e60653e99e4f6ae358a282e28...
==========

[commit-bot: I haz the power, 2017-02-28 21:07:55]

Committed patchset #5 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/6a6e60653e99e4f6ae358a282e28...

[Sergey Ulanov, 2017-02-28 22:27:57]

A revert of this CL (patchset #5 id:180001) has been created in
https://codereview.chromium.org/2719403005/ by sergeyu@chromium.org.

The reason for reverting is: Broke android build:
https://uberchromegw.corp.google.com/i/chromium.linux/builders/Android%20Builder.