Peter - you're probably most familiar with this area given recent changes to it. Can you take a look?

+peter, owencm

Elliot just told me that he got some reports of this from Twitter (i.e. it's happening for non-WebAPKs too.)

Note that this could be a Twitter bug. They have full control of what tabs they open, how they navigate them etc. I'd recommend first looking at Twitter's SW logic and ruling that out.

Twitter says this appears to be a browser bug, and it also doesn't appear to be WebApk specific.

I am still looking into this. Haven't found anything interesting yet

- It looks like twitter does not allow users to setup "Web Notifications" on Android anymore
- On Desktop with Chrome Canary, clicking on a DM Web Notification opens a new tab with the destination of the push notification

- I tested navigator.clients.openWindow() and client.navigate() on Android and they both navigate to the target URL
- I created a test page which allows users to register an arbirtrary service worker to listen for notifications (This should make it easy for others to repro my results) at  https://webapk-test.appspot.com/static/custom_service_worker/index.html (I am new to service worker stuff. Sometimes my test page does not unregister the service worker properly)

Assigning this bug to Peter Beverloo who knows a lot more about service workers and push than I do.

I looked at https://twitter.com/push_service_worker.js and I didn't see anything obviously wrong. However, I don't know the service worker JavaScript APIs very well

I repro on my device FYI in case anyone needs me to try anything.

Weirdly I've found that tapping Twitter notifications often open me to a random page on Twitter that I was on ~a week ago. Not sure what the deal is.

I also found this issue with Facebook today so I agree this is a WebAPK issue, not a Twitter issue.

> I also found this issue with Facebook today so I agree this is a WebAPK issue, not a Twitter issue.

It's not possible to get a Facebook webapk (they aren't installable) so that doesn't sound right. Perhaps this was from an early trial webapk when we didn't enforce installability checks? If so, I'd uninstall it and switch to the homescreen shortcut cause you probably aren't getting updates

I have a Facebook WebAPK installed. They use SW for notifications so with that turned on I think they passed the installability checks from when I did it.

Also note that I've received a number of updates to it (I've noticed it more than any as having regular update notifications).

Oops, my mistake! I had thought I tried it with notifications on and still couldn't get a webapk but I see it working now - sorry.

Good news I can repro the bug. I am unsure why it occurs though

I have a fix for this bug locally. It has to do with WebappDelegateFactory#activateContents()

For WebAPKs WebappDelegateFactory#activateContents() navigates the WebAPK and it should not do so
I think that twitter.com uses postMessage() to do the navigation and there was a race between the navigation done by WebappDelegateFactory#activateContents() and the navigation done as a result of postMessage()

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dcab9933793a5bb76c6627541211f86c20d6a5b2

commit dcab9933793a5bb76c6627541211f86c20d6a5b2
Author: pkotwicz <pkotwicz@chromium.org>
Date: Wed May 17 04:40:29 2017

[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}

[modify] https://crrev.com/dcab9933793a5bb76c6627541211f86c20d6a5b2/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/dcab9933793a5bb76c6627541211f86c20d6a5b2/chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
[modify] https://crrev.com/dcab9933793a5bb76c6627541211f86c20d6a5b2/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDelegateFactory.java
[modify] https://crrev.com/dcab9933793a5bb76c6627541211f86c20d6a5b2/chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkNavigationClient.java
[modify] https://crrev.com/dcab9933793a5bb76c6627541211f86c20d6a5b2/chrome/android/webapk/shell_apk/shell_apk_version.gni
[modify] https://crrev.com/dcab9933793a5bb76c6627541211f86c20d6a5b2/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java

We'll want to pull this onto 59 (and also roll out shell v5)

Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Please merge your CL to M59 release branch (3071) before 5PM PT tomorrow so we can pick it for this week's beta release. Thanks.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/157c3636e9b9cf67981e699ada98a03432056210

commit 157c3636e9b9cf67981e699ada98a03432056210
Author: Peter Kotwicz <pkotwicz@google.com>
Date: Tue May 23 15:42:21 2017

Merge 59: [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-Original-Commit-Position: refs/heads/master@{#472327}
Review-Url: https://codereview.chromium.org/2901013002 .
Cr-Commit-Position: refs/branch-heads/3071@{#671}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/157c3636e9b9cf67981e699ada98a03432056210/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/157c3636e9b9cf67981e699ada98a03432056210/chrome/android/java/src/org/chromium/chrome/browser/ServiceTabLauncher.java
[modify] https://crrev.com/157c3636e9b9cf67981e699ada98a03432056210/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDelegateFactory.java
[modify] https://crrev.com/157c3636e9b9cf67981e699ada98a03432056210/chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/WebApkNavigationClient.java
[modify] https://crrev.com/157c3636e9b9cf67981e699ada98a03432056210/chrome/android/webapk/shell_apk/shell_apk_version.gni
[modify] https://crrev.com/157c3636e9b9cf67981e699ada98a03432056210/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java

Verified fix with steps in #0 on 60.0.3107.3. Thanks!

Verified in 59.0.3071.71. Thanks!