Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 14 users
Status: Fixed
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocked on:
issue 561086
issue 594872

Blocking:
issue 599313



Sign in to add a comment
Pressing a notification should open the site standalone if it was added to the user's home screen with display set to standalone
Project Member Reported by owe...@chromium.org, Oct 9 2015 Back to list
What steps will reproduce the problem?
1. Add a site to your homescreen, and observe how it runs in standalone mode and appears app-like
2. Receive a notification from the site, tap to open it
3. Observe how it opens in a "tab" with an omnibox, rather than standalone

We should instead use the new database Lalit built to track when a site was last opened from the homescreen to launch the site in standalone if it has been launched as standalone from the home screen in the last 14 days.

 
I think we need sign-off from:
- privacy: because we are leaking the fact that the website was in the homescreen recently (though, if the user clears website data, we clear the database).
- security: because opening in standalone mode is "privileged" given that there is no Chrome UI, thus has security implications.

Also, the bug mentions notifications but do we want that for all links or only page opened from a notification? If I click on an example.com link, should I go to the standalone web app? (I would say yes) What if I type example.com in the URL bar? (I would like to see how it looks like)

Finally, you mention a 14 days period. Should opening an app from something else than the homescreen reset that period or not?
Status: Available
Comment 3 by engedy@chromium.org, Oct 13 2015
Labels: Cr-Privacy
 Issue 466652  has been merged into this issue.
Labels: Cr-UI-Browser-AppShortcuts
Cc: mattgaunt@chromium.org
Comment 7 by owe...@chromium.org, Nov 21 2015
This is something that came up a number of times at CDS and is a strong request from Flipkart.

If I lead on privacy and security approval, is someone willing to implement this change?
I've got higher priorities.
I'm happy to implement.
Owner: owe...@chromium.org
Status: Assigned
Great, thanks Dominick! I'll start on the security and privacy work in that case and will share with you once I have something together.
Blockedon: chromium:561086
Cc: jinho.b...@samsung.com
Labels: Cr-UI-Notifications
Labels: Hotlist-InstallableWeb
Update: I've asked security to assign a preliminary reviewer - if they are basically happy we can return to implementing.
Preliminary security lgtm, so we can proceed with implementing.
Cc: -dominickn@chromium.org owe...@chromium.org
Owner: dominickn@chromium.org
Great. Dominick - is this something you anticipate picking up this quarter?
The intention is that I'll have this implemented by the end of quarter. :)
Project Member Comment 19 by bugdroid1@chromium.org, Mar 25 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6e400421b116644b4e8b23c29fd2e0c3886af3af

commit 6e400421b116644b4e8b23c29fd2e0c3886af3af
Author: dominickn <dominickn@chromium.org>
Date: Fri Mar 25 02:02:47 2016

Store URLs in WebappDataStorage, and purge them when history is cleared.

Sites which have been added to homescreen on Android should be
allowed to open in standalone mode in response to push notifications. As
Chromium cannot detect what sites are on the homescreen after they
have been added, the heuristic is sites which have been launched from
homescreen within the last ten days will be opened in standalone from
a tap on a notification.

This CL lays the groundwork for this feature by storing webapp URLs
in WebappDataStorage. This is needed to be able to look up the last used
time, which is currently persisted in WebappDataStorage.

The last used time and webapp URL combined are history, so this
CL also resets both pieces of data when the user clears their history.
This requires further changes to allow null values in these fields. The
last used time is set appropriately when the app is next launched from
homescreen; the URL is reset if it is empty when the app is launched.
Additional tests for the behaviour introduced in this CL are added.

BUG= 541711 , 570579 

Review URL: https://codereview.chromium.org/1749603002

Cr-Commit-Position: refs/heads/master@{#383223}

[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestBase.java
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/browser/android/banners/app_banner_data_fetcher_android.cc
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/browser/android/shortcut_helper.cc
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/browser/android/shortcut_helper.h
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/browser/android/webapps/webapp_registry.cc
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/browser/android/webapps/webapp_registry.h
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/browser/browsing_data/browsing_data_remover.cc
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/6e400421b116644b4e8b23c29fd2e0c3886af3af/chrome/browser/browsing_data/browsing_data_remover_unittest.cc

Apologies - I'm going to play devils advocate because I want this feature but have some concerns over the current proposal.

This bug was around the issue that there is an inconsistency between how the the experience get after a user interacts with a notification compared to interaction via the add to homescreen launch.

This heuristic feels like it'll add  a deeper (less obvious) inconsistency - i.e. sometimes the notification shows standalone and sometimes it's in the browser (i.e. depending on the last time the user opened the web app).

Would it be worth blocking this bug on the issue relating to Chrome not knowing when a web app is added to homescreen, removing the need for this heuristic?
Devil's advocacy always welcome Matt! I agree with you that this pushes the inconsistency deeper, but as you note it will be less obvious to users.

Realistically we won't be able to know about the icon for sure until a very large complex project is complete, and even then it won't work for all sites using notifications, only those added to home screen.

I suggest we go ahead with this - see how it feels to us and look at the metrics to determine how well it's working overall. If the data indicates it's causing sites to randomly open in the different modes then we can always back pedal, but I really hope it will turn out to work effectively!
Blocking: 599313
@mattgaunt Matt I just liked to crbug.com/599313.  Physical Web would like to have PSA's which have been added to Home Screen to automatically open full screen when we fire an Intent into Chrome.

Personally, I don't think Notification taps, incoming Intents, and etc, should all need to have custom checks to handle this.  Chrome navigation code should just do the right thing from all incoming paths, if possible, imho.

The navigation code already checks if we should navigate a tab or to Intent out into a native app automatically, I think adding support for PSA's should go there.

That issue would certainly be blocked on Chrome knowing if a web app is added to homescreen -- I don't have any context on that issue, just going off your comment.
Just had a chat with a team here who explained the context.  I think the current hack of last-launched < 10 days is unfortunate but a good workaround until we have a better solution available.

I still think we can hide the details of how we implement "isAppAddedToHomescreen", improve it later, and change the Navigation code now.

If its a matter of priority/cycles, I may be able to take a stab at this (or find someone to) if you agree with the approach:

Should all navigations, from Chrome, from Notifications, from Intents, open app in full-screen if the app is added to homescreen?  Note that we do this for apps.
Comment 25 Deleted
[corrected link from deleted comment]

If it's OK with everyone I'd like to keep this issue focused on the notifications feature, and use the broader issue (https://bugs.chromium.org/p/chromium/issues/detail?id=599313) to discuss how to solve it in the general case.
Project Member Comment 27 by bugdroid1@chromium.org, Apr 8 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ce7d2f020a7452bd0399dd50e4580981deeb4dd

commit 1ce7d2f020a7452bd0399dd50e4580981deeb4dd
Author: dominickn <dominickn@chromium.org>
Date: Fri Apr 08 03:10:04 2016

Enable deep-linking from notifications for recently used web apps on the Android home screen.

This CL permits notifications pointing to a recently used,
standalone-capable web app on the Android home screen to open in the
standalone web frame.

The web app must be added to home screen, capable of opening in
standalone mode, and have been opened from the home screen within the
last ten days. If any of these conditions are not met, the notification
will fall back to the existing behaviour.

The search for a matching web app is conducted by comparing the URL
which the notification is asking to be opened with the scope of all web
apps known to Chromium. The scope for a web app is its URL, minus the
final component of its path, any query parameters, and any fragments.
A future CL will implement the manifest scope property, which will
replace this estimated scope. The web app with the longest scope that
matches the URL to be opened is chosen.

Additional tests for the scoped search are added in this CL. A new
shortcut source is added to indicate launches from a notification, and
the WebappRegistry is updated to not change the last updated time of web
app data which has not been opened from the home screen. This is
necessary to ensure web apps cannot spam the user with notifications to
update their last used time and hence how long Chromium will cache their
app data.

BUG= 541711 , 594872 
R=dfalcantara

Review URL: https://codereview.chromium.org/1867543002

Cr-Commit-Position: refs/heads/master@{#385967}

[modify] https://crrev.com/1ce7d2f020a7452bd0399dd50e4580981deeb4dd/chrome/android/java/src/org/chromium/chrome/browser/ChromeServiceTabLauncher.java
[modify] https://crrev.com/1ce7d2f020a7452bd0399dd50e4580981deeb4dd/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/document/TabDelegate.java
[modify] https://crrev.com/1ce7d2f020a7452bd0399dd50e4580981deeb4dd/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/1ce7d2f020a7452bd0399dd50e4580981deeb4dd/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/1ce7d2f020a7452bd0399dd50e4580981deeb4dd/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappInfo.java
[modify] https://crrev.com/1ce7d2f020a7452bd0399dd50e4580981deeb4dd/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappLauncherActivity.java
[modify] https://crrev.com/1ce7d2f020a7452bd0399dd50e4580981deeb4dd/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java
[modify] https://crrev.com/1ce7d2f020a7452bd0399dd50e4580981deeb4dd/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
[modify] https://crrev.com/1ce7d2f020a7452bd0399dd50e4580981deeb4dd/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/1ce7d2f020a7452bd0399dd50e4580981deeb4dd/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java
[modify] https://crrev.com/1ce7d2f020a7452bd0399dd50e4580981deeb4dd/chrome/browser/android/shortcut_info.h
[modify] https://crrev.com/1ce7d2f020a7452bd0399dd50e4580981deeb4dd/tools/metrics/histograms/histograms.xml

Project Member Comment 28 by bugdroid1@chromium.org, Apr 11 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b4abc9d04c20c2d83124a1b9cc3960e8f4e48bdb

commit b4abc9d04c20c2d83124a1b9cc3960e8f4e48bdb
Author: dominickn <dominickn@chromium.org>
Date: Mon Apr 11 14:40:48 2016

Remove the first launch from home screen requirement for notification deep linking.

https://crrev.com/1867543002 implemented notification deep linking for
Android, with the requirement that a web app be explicily launched from
home screen before it would be permitted to have deep links from
notifications. This CL removes this requirement. Now sites which have
been added to homescreen OR launched from homescreen within the
last ten days are permitted to be deep-linked from a notification.

BUG= 541711 

Review URL: https://codereview.chromium.org/1872983002

Cr-Commit-Position: refs/heads/master@{#386381}

[modify] https://crrev.com/b4abc9d04c20c2d83124a1b9cc3960e8f4e48bdb/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/b4abc9d04c20c2d83124a1b9cc3960e8f4e48bdb/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/b4abc9d04c20c2d83124a1b9cc3960e8f4e48bdb/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/b4abc9d04c20c2d83124a1b9cc3960e8f4e48bdb/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java

Labels: -Hotlist-InstallableWeb hotlist-InstallableWeb Merge-Request-51
Requesting merge to M51 for #28. This is a very minor trigger change for notification deep linking requested from the feature's PM. Merging will prevent an extra SharedPreference member being added to Android and then subsequently ignored from M52 onwards. Thanks!
Comment 30 by tin...@google.com, Apr 15 2016
Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)
Project Member Comment 31 by bugdroid1@chromium.org, Apr 15 2016
Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0e50cdbeee971718a29c199d76abc10dd6c43e04

commit 0e50cdbeee971718a29c199d76abc10dd6c43e04
Author: Dominick Ng <dominickn@chromium.org>
Date: Fri Apr 15 02:28:37 2016

Remove the first launch from home screen requirement for notification deep linking.

https://crrev.com/1867543002 implemented notification deep linking for
Android, with the requirement that a web app be explicily launched from
home screen before it would be permitted to have deep links from
notifications. This CL removes this requirement. Now sites which have
been added to homescreen OR launched from homescreen within the
last ten days are permitted to be deep-linked from a notification.

BUG= 541711 

Review URL: https://codereview.chromium.org/1872983002

Cr-Commit-Position: refs/heads/master@{#386381}
(cherry picked from commit b4abc9d04c20c2d83124a1b9cc3960e8f4e48bdb)

Review URL: https://codereview.chromium.org/1893513002 .

Cr-Commit-Position: refs/branch-heads/2704@{#67}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/0e50cdbeee971718a29c199d76abc10dd6c43e04/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
[modify] https://crrev.com/0e50cdbeee971718a29c199d76abc10dd6c43e04/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/0e50cdbeee971718a29c199d76abc10dd6c43e04/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/0e50cdbeee971718a29c199d76abc10dd6c43e04/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java

Status: Fixed
Comment 33 Deleted
Sign in to add a comment