Known issue. This is the time where we wait for the SW to startup so we can fire the event into it and it can imperatively trigger a tab open.

Does this feel meaningfully slower on Android O than before? If so we should dig in, but if not (or it's simply very variable) then I'd put this down to SW startup time.

cc peter in case there is any other reason this could be slow, or in case he has any suspicions about O regressions

This does seem like 2x worse recently. I've started counting and it seems more like 10 seconds. Whether it's O or something else, I have no idea.

The time to start the SW is just the time to start chrome browser+renderer process, right? If so, are we sure there's no room left to optimize that?

We should dig into the Twitter code (and/or ask them) about their logic here. They can delay the process on arbitrary work (network fetch. e.g.) which they probably shouldn't do.

Just read through their code, doesn't look like they're doing anything bad.

In response to a notification click they basically close the notification and then do this:

return c.getBestClient().then(function(e) {
                return e && e.focus ? e.focus().then(function() {
                    return e.postMessage({
                        type: l.ACTION_NAVIGATE,
                        payload: t
                    })
                }) : self.clients.openWindow(t)
            })

Looks to me like the issue is on our side.

I've noticed that too. It could be O (or a coincidence).

Do we have any metrics capturing the time from notification to X (e.g. where X captures each important stages: browser startup, ..., service worker startup, event handled) ?

Service worker startup is a likely culprit but regular 10 second delays sounds like something is very wrong and regressed.

The time to start the SW is not just browser+renderer process creation. Even when the browser and renderer are already created it can take seconds to create a SW on Android (thread contention in browser/renderer process, worker thread creation time, disk access all contribute to this). We are optimizing SW startup at issue 561209.

Here is some UMA for how long it takes to startup a service worker in response to a notification click event:
https://uma.googleplex.com/p/chrome/timeline_v2/?sid=b342fdb79a2078f333416878c8ca0d47

It's showing more like <1 second at median and 2-3 seconds at 95% when the browser process or renderer process also had to be created.

This UMA is recorded from the beginning of SW startup. It'd useful to add UMA for how long it took between tapping the notification and the beginning of SW startup.

Is it only Twitter people have noticed this on?

It'd also be good to test this on a toy website. My basic push notification demo site doesn't show a large delay with Android 7.0 (I don't have a device with 8.0). If someone brings their phone to me I can test.

Would like to ensure we isolate and understand this. Are we still blocked on an Android 8.0 device?

I just tested with Android Go and saw 2-3 seconds delay on my test site which seemed not greatly egregious for this device but still slower than launching the Chrome browser is. Repro steps:
- Go to https://alarming-web.appspot.com/index.html
- Subscribe
- On any device, go to https://alarming-web.appspot.com/notify to get a notification
- Close Chrome, try to open other apps to make sure Chrome is dead.
- Tap the notification.

There's a big spike on dev starting in late September. Is that just noise?

https://uma.googleplex.com/p/chrome/timeline_v2/?sid=ebc9fdd477db1b11ee4a9cc4b03797d6

I'm only seeing this on twitter from notifications, but that's the only PWA with notifications that I use, so I don't know if it's just twitter or not.

 Issue 770724  has been merged into this issue.

Do we have traces? Can someone post them here?

See also issue 600707: it's a known issue that we're creating two renderer processes for a notificationclick but still shouldn't cause 10 second delays.

The notification first lets Chrome up, we're not sure if there's a way to capture traces for this one.  I heard there might be some scheduler issues in Android, we'll try reaching out to Android folks as well.

(blink-worker bug triage) Looks like awdf@ is investigating this?

I'm investigating notifications not showing up when the device is idle on O over at Issue 777106 - not sure whether this is related since the device can't be idle when a notification is tapped, but maybe there's a similar root cause if this is O-specific too, related to the new background check restrictions.

awdf: I just discovered internal bug b/67675286 and it looks like a probable cause of this? Or is it thought to be different somehow?

Good point, yes it could well be the same thing. I will keep pursuing that internal bug!

Meanwhile if anybody else experiences notification taps being laggy, a logcat dump (starting from the sysui notification_clicked log) would be really helpful.

Thanks.

Thanks awdf@! I saw an interesting update on the internal bug. WDYT? Does it make sense to you, should we loop in someone else?

@kenji - yes the suggestion on the bug makes sense to me, I don't know why we weren't setting that intent flag already. I'm landing a patch to set the flag as suggested at crrev.com/r/750981 - let's hope this fixes things!

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

commit 42af1f7909b1a4cc70543d43f0a9a6edd226042e
Author: Anita Woodruff <awdf@chromium.org>
Date: Thu Nov 02 14:31:56 2017

[Android Notifications] Add foreground flag to notification intents

- Adding this FLAG_RECEIVER_FOREGROUND flag should ensure that
the intent is delivered to our broadcast receiver immediately after
the notification is clicked or closed.

- This should hopefully fix the bug where tapping a web notification
sometimes does nothing for several seconds when Chrome is in the
background.

Bug:  764437 
Change-Id: I31d92266f31516acff896379647f5e65ae8435a4
Reviewed-on: https://chromium-review.googlesource.com/750981
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513484}
[modify] https://crrev.com/42af1f7909b1a4cc70543d43f0a9a6edd226042e/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java

This fix is hard to verify because the bug is flaky, but going to optimistically close this as fixed, please reopen if anyone sees this occurring after the next Chrome release (64). 

Awesome, thanks awdf!

Have you considered merging to 63? If Android O is already going to users we should probably get the fix in 63 if possible.

Good idea Matt. 

Branch point was a while ago, but this is a 1-line fix that just sets a flag on an intent, so I'm pretty confident of its safety. And it fixes a pretty bad bug (effectively an ANR on some notification taps).

Requesting merge for M63 for these reasons.

This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

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

What could potentially go wrong if there was a logic issue with this change? 

There's no chance of a logic error per se because this change simply sets a flag on all web notification click (and close) intents.

However this flag does affect how Android treats the code receiving the intent, giving it less time to run, on all versions of Android. We mitigate this by scheduling a job (on N+) which gives us more time to run despite the tougher restrictions on these Android versions.

So the worst case would be that some web notifications clicks / dismissals do nothing if our code to dismiss / activate the notification does not run to completion within Android's time limit.

On the other hand, not making this change means some notification clicks / dismissals take many seconds or even over a minute before they have any user-visible effect. We've definitely seen this happen in the wild and seems to be a big problem on O.

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

commit 53b7d7184b8d149c3365ef6683125c5bb7c77f86
Author: Peter Beverloo <peter@chromium.org>
Date: Mon Nov 06 19:18:54 2017

Only set the FLAG_RECEIVER_FOREGROUND flag for Web Notifications on >=N

We understand what this behaviour does in cases where the Job
Scheduler is used to deliver Web Notification events, but aren't
sure of its behaviour on older versions of Android as it influences
the time we're allowed to run for. Limit it to >= Android N in
order to reduce the risk.

R=awdf
BUG= 764437 

Change-Id: Ib398adb3ac2ff61a93282639956e092d8315fb57
Reviewed-on: https://chromium-review.googlesource.com/755194
Commit-Queue: Peter Beverloo <peter@chromium.org>
Reviewed-by: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514203}
[modify] https://crrev.com/53b7d7184b8d149c3365ef6683125c5bb7c77f86/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java

Based on further discussion we decided to only set this flag for N+, where we know it to be safe, giving us additional time to investigate its impact on older versions of Android.

cmasso@: we'll verify this in the upcoming Canary again. Could it be considered for merge assuming that it all works fine there?

is this a regression in M63? I want to be a little more conservative with M63 since this build will be on the Play Store during the holidays so we want to avoid as much as possible merging anything that is not critical but could eventually cause a breakage. 

We believe that it's a interaction experience regression in Android O, not one tied to a particular Chrome release. The number of O users will presumably grow significantly over the holiday period, as more devices with the operating system version are to be released.

Any results from canary? About 18% of our users could be affected in case something goes wrong with this change (which is better than 100% of users but still a lot though). How confident are we with this fix?

We don't get a lot of data from Canary but our notifications UMA shows we are still receiving around the same number of notification clicks since this change went out in 64.0.3257.0 - https://uma.googleplex.com/p/chrome/timeline_v2/?sid=2c5d5f24a1989cab592d5cdc5e1b945a.


If we want to be even more conservative one option would be to restrict the change to only users on Android O which I imagine is much less than 10% of the userbase, then monitor this in Dev/Beta.

Spoke with peter@ about this and we agreed that merging the fix just on O is a good compromise especially given the bug has mostly been reported on O.

https://chromium-review.googlesource.com/c/chromium/src/+/764148 to restrict to O is in flight. 

Some jobs are failing. Please take a look and resolve the issue.

Ah that's unfortunate - I'm off corp now and don't have access to check which bots failed till Monday, will check first thing then. 

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

commit f9b078d0644e0e84044ede407c32809e1082149e
Author: Anita Woodruff <awdf@chromium.org>
Date: Mon Nov 13 11:37:13 2017

[Android] Restrict notification intent foreground flag to O+

- Only set the FLAG_RECEIVER_FOREGROUND flag on notification clicks
and closes when the sdk version is >= O.

Bug:  764437 
Change-Id: I8a51f7cc7db670700ced543848c4bda092bf2248
Reviewed-on: https://chromium-review.googlesource.com/764148
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515927}
[modify] https://crrev.com/f9b078d0644e0e84044ede407c32809e1082149e/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java

Merge approved! branch 3239. Please make sure to verify the fix after merging it.

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

commit 988a73ba38712f2ea23c4c49e4da78305fc8a806
Author: Anita Woodruff <awdf@chromium.org>
Date: Tue Nov 14 15:46:08 2017

[Android Notifications] Add foreground flag to notification intents

- Adding this FLAG_RECEIVER_FOREGROUND flag should ensure that
the intent is delivered to our broadcast receiver immediately after
the notification is clicked or closed.

- This should hopefully fix the bug where tapping a web notification
sometimes does nothing for several seconds when Chrome is in the
background.

Bug:  764437 
Change-Id: I31d92266f31516acff896379647f5e65ae8435a4
Reviewed-on: https://chromium-review.googlesource.com/750981
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#513484}(cherry picked from commit 42af1f7909b1a4cc70543d43f0a9a6edd226042e)
Reviewed-on: https://chromium-review.googlesource.com/768688
Reviewed-by: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#480}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/988a73ba38712f2ea23c4c49e4da78305fc8a806/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java

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

commit b34a1d448c47de5161fb5e0866e33d3837f48b3e
Author: Peter Beverloo <peter@chromium.org>
Date: Tue Nov 14 15:47:55 2017

Only set the FLAG_RECEIVER_FOREGROUND flag for Web Notifications on >=N

We understand what this behaviour does in cases where the Job
Scheduler is used to deliver Web Notification events, but aren't
sure of its behaviour on older versions of Android as it influences
the time we're allowed to run for. Limit it to >= Android N in
order to reduce the risk.

R=​awdf
BUG= 764437 

Change-Id: Ib398adb3ac2ff61a93282639956e092d8315fb57
Reviewed-on: https://chromium-review.googlesource.com/755194
Commit-Queue: Peter Beverloo <peter@chromium.org>
Reviewed-by: Anita Woodruff <awdf@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#514203}(cherry picked from commit 53b7d7184b8d149c3365ef6683125c5bb7c77f86)
Reviewed-on: https://chromium-review.googlesource.com/768888
Cr-Commit-Position: refs/branch-heads/3239@{#481}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/b34a1d448c47de5161fb5e0866e33d3837f48b3e/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java

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

commit 84f43f3cecb3040936a49036b32d911f505609d2
Author: Anita Woodruff <awdf@chromium.org>
Date: Tue Nov 14 15:48:48 2017

[Android] Restrict notification intent foreground flag to O+

- Only set the FLAG_RECEIVER_FOREGROUND flag on notification clicks
and closes when the sdk version is >= O.

Bug:  764437 
Change-Id: I8a51f7cc7db670700ced543848c4bda092bf2248
Reviewed-on: https://chromium-review.googlesource.com/764148
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#515927}(cherry picked from commit f9b078d0644e0e84044ede407c32809e1082149e)
Reviewed-on: https://chromium-review.googlesource.com/768689
Reviewed-by: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#482}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/84f43f3cecb3040936a49036b32d911f505609d2/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java

Merged, thanks.

Will verify it works as intended once these 3 cherry-picks make it into a build of 63.

The NextAction date has arrived: 2017-11-15

Cherry-picks landed in 63.0.3239.51.

Verified notification taps still work on Oreo and the intents are now received in the foreground. 

(Verified by tapping on a notification and checking the intent appears in the '[foreground]' section of historical broadcasts using adb:

  $ watch -n 1 "adb shell dumpsys activity broadcasts | grep 'summary \[background\]\|summary \[foreground\]' -A15"

)