New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 596668 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Loosen up the swipe away behavior for notifications

Project Member Reported by scottj@chromium.org, Mar 21 2016

Issue description

If the user swipes away a Physical Web notification nothing shows up until all current beacons are lost. This protects the user from repeated notifications but penalizes people in Mall, and more importantly conferences, where people can't escape all beacons.

The goal of this switch is to keep from pestering the user who swipes the notification away but still brings back the notification if a properly new beacon actually shows up.


 

Comment 1 by scottj@chromium.org, Mar 21 2016

One solution is to keep a record of the last X beacons seen, with timestamps. As beacons can easily be in range and be lost/found again, any 'found' beacon would be compared to this list. If already there, don't notify. Any truly new beacon would not be in this list, which would then trigger the notification.

As a first pass suggestion, have the comparison for old beacons be 'been seen within 60 minutes' This will keep existing beacons from showing up too quickly yet allow walking up to a new device to properly notify the user.
We should also have a "max notify" cap beyond which we won't show these notifications. If we do end up firing notifications even after a user swipes, we need to be careful not to send too many additional notifications.

Comment 3 by scottj@chromium.org, Mar 22 2016

Much of this could be mitigated by having the time to reset to be fairly high. The goal here is to see new beacons, not bring back old ones. Setting the window to be fairly large keeps this fix much simple *and* reduces the annoyance factor considerably.

Comment 4 by mmo...@chromium.org, Mar 29 2016

Another thing to consider: Persistent (non-swipeable) notifications.

The current notification is really a "state" your phone is in, and you cannot really change your state, so maybe the notification shouldn't go away.

Obviously we would want a way for the user to remove the notification if they really wanted, but perhaps more like this:
- support URL blacklisting/muting
- support "silence for ... minutes/hours/days" like calendar/messaging apps
- support disabling notifications
I worry that many users will see this as spammy -- even though it's technically a state, users don't typically respond well to non-dismissable notifications (the last Google product I heard that did this was Now with their traffic/weather information, and they didn't see good engagement)

Comment 6 by scottj@chromium.org, Mar 31 2016

I'd like to keep this simple, we're just trying to tweak the existing behavior as we're getting lots of reports of people swiping away the notification accidentally and then never seeing any more alerts.

This tweak is simply to make sure that when new beacons show up, they trigger the notification.

Comment 7 by mmo...@chromium.org, Mar 31 2016

Status: Available (was: Untriaged)
Do we want to remember any beacon that was swiped away ever?

Or is it enough to say: if there is a new beacon that wasn't seen when you swiped away last time, show again?

If the latter, which I think should be enough, then I think we only need to keep one short list around in SharedPreferences: the set of URLs that we thought were nearby when a notification was last swiped away.

Every new onFound event just checks if it was in that list, and we ignore it if it was.

This way we can just reset the list every time a new Notification is shown, without having to deal with timeouts or cleanup.

Does this sound OK?

Conley/Matt, anyone want to take ownership?
I think the latter is fine, although it does mean if you go back and forth between two places that have beacons, you could constantly be getting the notification (although I think this is rare today).

Is remembering every beacon that was swiped away within the last X hours / days much more complex? I do think that would be ideal implementation, but don't have a sense of how much more complex it is to implement.

Comment 9 by scottj@chromium.org, Mar 31 2016

Matt already put a bit of work into this I think his rough logic was:
1) When we notify user, cache the currently seen URLs
2) Whenever a new onFound comes in, ignore if it is in the cached list
3) If not, notify user (assuming it was swiped away previously)
4) There is a timeout on this cache (TBD)
5) Which means after timeout, a previous beacon and again notify the user
6) So we need to set the timeout long enough to not pester the user

I'm sure there are edge cases I'm missing but it captures the basic idea, only notify if a new beacon is found.
That plan sounds fine, a reasonable timeout will address my concerns.
Yeah, this is strictly better than today, so any reasonable timeout SGTM
Labels: M-52
Project Member

Comment 13 by bugdroid1@chromium.org, May 10 2016

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

commit a42232196abfc73d0c710664595af8e1ff01146a
Author: cco3 <cco3@chromium.org>
Date: Tue May 10 02:04:34 2016

Store scan timestamps with Physical Web URLs

This change adds a timestamp to UrlInfo so that we can tell how long
ago the URL was last seen.  This will permit us to do smarter caching.

BUG= 598106 , 596668 

Review-Url: https://codereview.chromium.org/1966493002
Cr-Commit-Position: refs/heads/master@{#392515}

[modify] https://crrev.com/a42232196abfc73d0c710664595af8e1ff01146a/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlInfo.java
[modify] https://crrev.com/a42232196abfc73d0c710664595af8e1ff01146a/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java

Project Member

Comment 14 by bugdroid1@chromium.org, May 10 2016

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

commit 3cb9951a5aa2edb978f434f31221b00bbc631ba0
Author: cco3 <cco3@chromium.org>
Date: Tue May 10 19:09:48 2016

Store Physical Web URL data in memory

Instead of repeatedly reading and deserializing nearby URLs and
resolved URLs, this change keeps them in memory.  (There are not
many URLs stored at one time, so memory shouldn't be an issue).
This also paves the way for implementing a smarter cache in the
UrlManager.

BUG= 598106 , 596668 

Review-Url: https://codereview.chromium.org/1894073006
Cr-Commit-Position: refs/heads/master@{#392671}

[modify] https://crrev.com/3cb9951a5aa2edb978f434f31221b00bbc631ba0/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebDiagnosticsPage.java
[modify] https://crrev.com/3cb9951a5aa2edb978f434f31221b00bbc631ba0/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java

Project Member

Comment 15 by bugdroid1@chromium.org, May 12 2016

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

commit f9f0e3cb412633e68d98b485c436998703f9b9dd
Author: cco3 <cco3@chromium.org>
Date: Thu May 12 18:02:18 2016

Extend the PhysicalWeb UrlManager cache

1. Instead of resetting the cache everytime the Physical Web starts,
this change only resets the cache when we explicitly call
stopPhysicalWeb().

2. This allows the cache to grow up to 100 URLs, but kicks all that
have been in the cache longer than 24 hours.

BUG= 598106 , 596668 

Review-Url: https://codereview.chromium.org/1971583002
Cr-Commit-Position: refs/heads/master@{#393299}

[modify] https://crrev.com/f9f0e3cb412633e68d98b485c436998703f9b9dd/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java
[modify] https://crrev.com/f9f0e3cb412633e68d98b485c436998703f9b9dd/chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java

Comment 17 by cco3@chromium.org, May 13 2016

Owner: cco3@chromium.org
Project Member

Comment 18 by bugdroid1@chromium.org, May 13 2016

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

commit a416f1b1baaf280fd829abd98e8016d2560c97a2
Author: cco3 <cco3@chromium.org>
Date: Fri May 13 19:36:54 2016

Simplify Physical Web UrlManager logic

UrlManager has several confusing methods that take parameters for
the number of previously displayable URLs and the number of
currently displayable URLs.  This is no longer necessary given that
we now store these values in member variables.  This change simplifies
these methods and their calling code.

BUG= 596668 

Review-Url: https://codereview.chromium.org/1977633002
Cr-Commit-Position: refs/heads/master@{#393605}

[modify] https://crrev.com/a416f1b1baaf280fd829abd98e8016d2560c97a2/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java
[modify] https://crrev.com/a416f1b1baaf280fd829abd98e8016d2560c97a2/chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java

Comment 19 by cco3@chromium.org, May 24 2016

Status: Started (was: Available)
Project Member

Comment 20 by bugdroid1@chromium.org, May 26 2016

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

commit 44ac07c8316385250002371bec549fc6621826ec
Author: cco3 <cco3@chromium.org>
Date: Thu May 26 00:55:39 2016

Change Physical Web notification display behavior

Currently, we only display a notification when we go from 0 to 1
displayable URLs.  The reason for this is that we don't want to
needlessly displayed a notification if the user recently swiped it
away.  However, in a URL dense environment (e.g. a mall filled with
beacons), a user may not ever go from seeing 0 to 1 displayable URLs,
since they will always be around multiple.  A single swipe away will
then leave them never seeing another notification.

This change displays a notification if we go from seeing 0 to 1
displayable URLs *or* if we see a displayable URL that does not exist
in our cache (i.e. a URL that has not been encountered for 24 hours.)

This behavior could be further refined, but this is a positive step
toward finding a balance between being there when desired and not
needlessly renotifying if a user has swiped away a notification.

BUG= 596668 

Review-Url: https://codereview.chromium.org/1969983002
Cr-Commit-Position: refs/heads/master@{#396060}

[modify] https://crrev.com/44ac07c8316385250002371bec549fc6621826ec/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlInfo.java
[modify] https://crrev.com/44ac07c8316385250002371bec549fc6621826ec/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/UrlManager.java
[modify] https://crrev.com/44ac07c8316385250002371bec549fc6621826ec/chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlInfoTest.java
[modify] https://crrev.com/44ac07c8316385250002371bec549fc6621826ec/chrome/android/javatests/src/org/chromium/chrome/browser/physicalweb/UrlManagerTest.java

Comment 21 by cco3@chromium.org, May 26 2016

Status: Fixed (was: Started)

Sign in to add a comment