[Android WebAPK] Don't navigate WebAPK as a result of WindowClient.focus()

[Android WebAPK] Don't navigate WebAPK as a result of WindowClient.focus()

This CL makes WindowClient.Focus() not navigate the WebAPK if the WebAPK is
already open.

When a user taps on a Twitter notification, the following race condition occurs:
- The service worker uses postMessage() and the main page navigates by setting
  window.location.href
- The service worker calls WindowClient.Focus(). This causes a navigation to
  the current page

This CL will remove the race by making WindowClient.Focus() not do any
navigation if the WebAPK is already running

BUG=711011
Review-Url: https://codereview.chromium.org/2849873002
Cr-Commit-Position: refs/heads/master@{#472327}
Committed: https://chromium.googlesource.com/chromium/src/+/dcab9933793a5bb76c6627541211f86c20d6a5b2

[pkotwicz, 2017-04-28 14:57:34]

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

[pkotwicz, 2017-04-28 14:57:35]

Xi, can you please take a look?

[pkotwicz, 2017-04-28 15:00:11]

Patchset #2 (id:20001) has been deleted

[Xi Han, 2017-05-01 19:04:05]

Thanks for fixing it! lgtm

[pkotwicz, 2017-05-03 01:28:58]

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

[pkotwicz, 2017-05-03 01:28:59]

Dominick for OWNERS

[dominickn, 2017-05-03 01:52:03]

webapps lgtm

[pkotwicz, 2017-05-04 19:17:11]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2017-05-04 19:17:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, hanxi@chromium.org
Link to the patchset: https://codereview.chromium.org/2849873002/#ps60001
(title: "Merge branch 'master' into twitter")

[commit-bot: I haz the power, 2017-05-04 19:17:59]

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 19:27:24]

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

[commit-bot: I haz the power, 2017-05-04 19:27:26]

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

[pkotwicz, 2017-05-05 20:47:49]

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

[pkotwicz, 2017-05-05 20:47:49]

Yaron for
	chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
OWNERS

[Yaron, 2017-05-08 16:51:47]

have you tested the case of inside a webapk, clicking a link to external app
(e.g. cause of url-shortened link) and then that redirecting back to the webapk?

https://codereview.chromium.org/2849873002/diff/60001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
(right):

https://codereview.chromium.org/2849873002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java:62:
Intent intent = WebApkNavigationClient.createLaunchWebApkIntent(
Hmm. i'm not thrilled with the idea that we have the notion of forcing
navigations but I don't have a better suggestion.

https://codereview.chromium.org/2849873002/diff/60001/chrome/android/webapk/s...
File chrome/android/webapk/shell_apk/shell_apk_version.gni (right):

https://codereview.chromium.org/2849873002/diff/60001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/shell_apk_version.gni:9:
template_shell_apk_version = 5
do we need to update the version number if we haven't yet shipped 4? you don't
want to block 4 changes on this rolling out?

[Yaron, 2017-05-10 17:56:17]

https://codereview.chromium.org/2849873002/diff/60001/chrome/android/webapk/s...
File chrome/android/webapk/shell_apk/shell_apk_version.gni (right):

https://codereview.chromium.org/2849873002/diff/60001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/shell_apk_version.gni:9:
template_shell_apk_version = 5
On 2017/05/08 16:51:47, Yaron (limited availability) wrote:
> do we need to update the version number if we haven't yet shipped 4? you don't
> want to block 4 changes on this rolling out?

ignore me. we pushed 4 :)

[pkotwicz, 2017-05-10 19:00:07]

If a user taps a link inside a WebAPK which is a URL shortener which points back
to the WebAPK we will temporarily show the minibar. However, this is an issue
with piotrs@ CL https://codereview.chromium.org/2829943002/. Thank you for
directing my attention at this issue

[Yaron, 2017-05-11 14:54:45]

On 2017/05/10 19:00:07, pkotwicz wrote:
> If a user taps a link inside a WebAPK which is a URL shortener which points
back
> to the WebAPK we will temporarily show the minibar. However, this is an issue
> with piotrs@ CL https://codereview.chromium.org/2829943002/. Thank you for
> directing my attention at this issue

lgtm

[Yaron, 2017-05-11 15:00:04]

https://codereview.chromium.org/2849873002/diff/60001/chrome/android/webapk/s...
File
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java
(right):

https://codereview.chromium.org/2849873002/diff/60001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java:101:
forceNavigation = getIntent().getBooleanExtra(
wait, actually. so this means there are times when a webapk is already loaded
and you get an intent with a url and we won't actually navigate it? that seems
wrong.

[pkotwicz, 2017-05-13 02:53:33]

Yaron can you please take another look?

https://codereview.chromium.org/2849873002/diff/60001/chrome/android/webapk/s...
File
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java
(right):

https://codereview.chromium.org/2849873002/diff/60001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java:101:
forceNavigation = getIntent().getBooleanExtra(
I tried sending an intent of type Intent.ACTION_MAIN and
Intent.CATEGORY_LAUNCHER and it does not seem to work on Kitkat.

The intent launches the WebAPK if it is not running already but it does not
bring the WebAPK to the foreground if it is running

[Yaron, 2017-05-15 15:24:36]

lgtm

[pkotwicz, 2017-05-15 18:01:14]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2017-05-15 18:02:43]

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-15 18:34:22]

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

[commit-bot: I haz the power, 2017-05-15 18:34:23]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Yaron, 2017-05-16 16:01:32]

On 2017/05/15 18:34:23, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
>
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

looks like bots hit a compile error. may need to rebase/update sources?

[pkotwicz, 2017-05-16 16:29:31]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2017-05-16 16:29:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, hanxi@chromium.org, yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/2849873002/#ps80001
(title: "Merge branch 'master' into twitter")

[commit-bot: I haz the power, 2017-05-16 16:30:07]

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-16 17:56:57]

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

[commit-bot: I haz the power, 2017-05-16 17:56:58]

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

[Yaron, 2017-05-16 18:37:30]

The CQ bit was checked by yfriedman@chromium.org

[commit-bot: I haz the power, 2017-05-16 18:38:56]

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-16 23:43:09]

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

[commit-bot: I haz the power, 2017-05-16 23:43:11]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[pkotwicz, 2017-05-17 01:11:09]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2017-05-17 01:12:20]

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-17 04:40:32]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1494983469880470,
"parent_rev": "d3e42cee3d26f0fc4730bd5d9e861a43e0fc739d", "commit_rev":
"dcab9933793a5bb76c6627541211f86c20d6a5b2"}

[commit-bot: I haz the power, 2017-05-17 04:41:05]

Description was changed from

==========
[Android WebAPK] Don't navigate WebAPK as a result of WindowClient.focus()

This CL makes WindowClient.Focus() not navigate the WebAPK if the WebAPK is
already open.

When a user taps on a Twitter notification, the following race condition occurs:
- The service worker uses postMessage() and the main page navigates by setting
  window.location.href
- The service worker calls WindowClient.Focus(). This causes a navigation to
  the current page

This CL will remove the race by making WindowClient.Focus() not do any
navigation if the WebAPK is already running

BUG=711011
==========

to

==========
[Android WebAPK] Don't navigate WebAPK as a result of WindowClient.focus()

This CL makes WindowClient.Focus() not navigate the WebAPK if the WebAPK is
already open.

When a user taps on a Twitter notification, the following race condition occurs:
- The service worker uses postMessage() and the main page navigates by setting
  window.location.href
- The service worker calls WindowClient.Focus(). This causes a navigation to
  the current page

This CL will remove the race by making WindowClient.Focus() not do any
navigation if the WebAPK is already running

BUG=711011

Review-Url: https://codereview.chromium.org/2849873002
Cr-Commit-Position: refs/heads/master@{#472327}
Committed:
https://chromium.googlesource.com/chromium/src/+/dcab9933793a5bb76c6627541211...
==========

[commit-bot: I haz the power, 2017-05-17 04:41:06]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/dcab9933793a5bb76c6627541211...