Bookmark apps on non-Chrome OS should not be fully synced |
||||||||
Issue descriptionWhen a bookmark apps is synced to a non-Chrome OS device it should be synced into a special state. Apps in this state will appear in chrome://apps but will otherwise be uninstalled: - the install banner checks should treat it as uninstalled - it should have no shortcuts added - it should run in a tab when launched - it should not have the 'open in window' option - it should not have the 'create shortcuts' option - it should have a new menu item in chrome://apps to install it properly 1-pager: https://docs.google.com/document/d/10cqN9geDhPB-K32p2wc1Omrsj82zxiUE1o16fJW-JVI/edit#
,
Aug 16
,
Aug 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9f94e0a7c98be9d44179a6c584025ff5b2d4f239 commit 9f94e0a7c98be9d44179a6c584025ff5b2d4f239 Author: Ben Wells <benwells@chromium.org> Date: Sat Aug 18 05:23:07 2018 Mark synced bookmark apps on non-Chrome OS as not locally installed. Bookmark apps that are not locally installed will not be treated as installed for the installation banner checks. In the future more changes to their behaviour, in relation to the chrome://apps page will be introduced. Bug: 874841 Change-Id: I48741967a646c6aac95cde3c832b8c8860b4f7b1 Reviewed-on: https://chromium-review.googlesource.com/1177529 Commit-Queue: Ben Wells <benwells@chromium.org> Reviewed-by: vitaliii <vitaliii@chromium.org> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Cr-Commit-Position: refs/heads/master@{#584304} [modify] https://crrev.com/9f94e0a7c98be9d44179a6c584025ff5b2d4f239/chrome/browser/banners/app_banner_manager_desktop.cc [modify] https://crrev.com/9f94e0a7c98be9d44179a6c584025ff5b2d4f239/chrome/browser/extensions/bookmark_app_helper.cc [modify] https://crrev.com/9f94e0a7c98be9d44179a6c584025ff5b2d4f239/chrome/browser/extensions/bookmark_app_helper.h [modify] https://crrev.com/9f94e0a7c98be9d44179a6c584025ff5b2d4f239/chrome/browser/extensions/bookmark_app_helper_unittest.cc [modify] https://crrev.com/9f94e0a7c98be9d44179a6c584025ff5b2d4f239/chrome/browser/extensions/browsertest_util.cc [modify] https://crrev.com/9f94e0a7c98be9d44179a6c584025ff5b2d4f239/chrome/browser/extensions/extension_sync_service.cc [modify] https://crrev.com/9f94e0a7c98be9d44179a6c584025ff5b2d4f239/chrome/browser/sync/test/integration/two_client_apps_sync_test.cc [modify] https://crrev.com/9f94e0a7c98be9d44179a6c584025ff5b2d4f239/chrome/browser/web_applications/extensions/BUILD.gn [add] https://crrev.com/9f94e0a7c98be9d44179a6c584025ff5b2d4f239/chrome/browser/web_applications/extensions/bookmark_app_util.cc [add] https://crrev.com/9f94e0a7c98be9d44179a6c584025ff5b2d4f239/chrome/browser/web_applications/extensions/bookmark_app_util.h
,
Aug 20
,
Aug 21
Ben: Does this fix the issue, or are there more CLs coming?
,
Aug 21
This gets the preference we need in, and fixes beforeInstallPrompt and other installability checks. Still needed is: - force launchContainer to be tab if not locally installed - update chrome://apps menus
,
Aug 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6dcbe65333cf26da1704ee2b56510195c255cf2d commit 6dcbe65333cf26da1704ee2b56510195c255cf2d Author: Ben Wells <benwells@chromium.org> Date: Fri Aug 24 12:59:09 2018 Force bookmark apps that weren't locally installed to open in tabs. This change causes all bookmark apps (including PWAs) which are not marked as locally installed to always open in tabs. Apps are marked as locally installed when they were not created on the machine but were synced to the machine, unless the device is Chrome OS - where all apps are marked as locally installed. This change does not affect hosted apps. Bug: 874841 Change-Id: Ia3936bb57e0fec94097fbf2925f676c97b703dc3 Reviewed-on: https://chromium-review.googlesource.com/1185336 Commit-Queue: Ben Wells <benwells@chromium.org> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Cr-Commit-Position: refs/heads/master@{#585813} [modify] https://crrev.com/6dcbe65333cf26da1704ee2b56510195c255cf2d/chrome/browser/extensions/bookmark_app_helper_unittest.cc [modify] https://crrev.com/6dcbe65333cf26da1704ee2b56510195c255cf2d/chrome/browser/extensions/launch_util.cc [modify] https://crrev.com/6dcbe65333cf26da1704ee2b56510195c255cf2d/chrome/browser/web_applications/extensions/bookmark_app_util.cc [modify] https://crrev.com/6dcbe65333cf26da1704ee2b56510195c255cf2d/chrome/browser/web_applications/extensions/bookmark_app_util.h
,
Aug 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e13d3169126389cf036e23f19bcf107dd7bc7652 commit e13d3169126389cf036e23f19bcf107dd7bc7652 Author: Ben Wells <benwells@chromium.org> Date: Fri Aug 31 00:22:33 2018 Don't treat bookmark apps that aren't locally installed as PWAs. This means if a PWA is installed on one machine and synced to another machine such that it isn't locally installed (i.e. it is synced to a non-Chrome OS machine), it won't be treated as an installed PWA. This affects features such as the navigation options available. Bug: 874841 Change-Id: I42667edbd7d9ac7dbe2a088d4b11bda422beee8b Reviewed-on: https://chromium-review.googlesource.com/1195382 Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Commit-Queue: Ben Wells <benwells@chromium.org> Cr-Commit-Position: refs/heads/master@{#587874} [modify] https://crrev.com/e13d3169126389cf036e23f19bcf107dd7bc7652/chrome/browser/extensions/bookmark_app_navigation_throttle_browsertest.cc [modify] https://crrev.com/e13d3169126389cf036e23f19bcf107dd7bc7652/chrome/browser/extensions/extension_util.cc [modify] https://crrev.com/e13d3169126389cf036e23f19bcf107dd7bc7652/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
,
Aug 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/72ecaf0c5162f100ceb3697b4ca49c26ad98bdf0 commit 72ecaf0c5162f100ceb3697b4ca49c26ad98bdf0 Author: Ben Wells <benwells@chromium.org> Date: Fri Aug 31 02:22:52 2018 Update chrome://apps for bookmark apps that aren't locally installed. When bookmark apps aren't locally installed, they should be dimmed out and they should have the launch type, app info and create shortcuts menus removed. This change also cleans up the logic that shows these menus and moved it to C++ where it is easier to reason about. Bug: 874841 Change-Id: Ia6af16a383c6f4434ac9da9e49882186c2c20015 Reviewed-on: https://chromium-review.googlesource.com/1195124 Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Commit-Queue: Ben Wells <benwells@chromium.org> Cr-Commit-Position: refs/heads/master@{#587920} [modify] https://crrev.com/72ecaf0c5162f100ceb3697b4ca49c26ad98bdf0/chrome/browser/resources/ntp4/apps_page.js [modify] https://crrev.com/72ecaf0c5162f100ceb3697b4ca49c26ad98bdf0/chrome/browser/resources/ntp4/page_list_view.js [modify] https://crrev.com/72ecaf0c5162f100ceb3697b4ca49c26ad98bdf0/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
,
Aug 31
,
Sep 3
,
Sep 4
Note this has been tested on Windows Canary.
,
Sep 4
Your change meets the bar and is auto-approved for M70. Please go ahead and merge the CL to branch 3538 manually. Please contact milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/04d51521948290eed20beb3c30e631d70f2ed8c5 commit 04d51521948290eed20beb3c30e631d70f2ed8c5 Author: Ben Wells <benwells@chromium.org> Date: Tue Sep 04 07:39:50 2018 Don't treat bookmark apps that aren't locally installed as PWAs. This means if a PWA is installed on one machine and synced to another machine such that it isn't locally installed (i.e. it is synced to a non-Chrome OS machine), it won't be treated as an installed PWA. This affects features such as the navigation options available. TBR=benwells@chromium.org (cherry picked from commit e13d3169126389cf036e23f19bcf107dd7bc7652) Bug: 874841 Change-Id: I42667edbd7d9ac7dbe2a088d4b11bda422beee8b Reviewed-on: https://chromium-review.googlesource.com/1195382 Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Commit-Queue: Ben Wells <benwells@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#587874} Reviewed-on: https://chromium-review.googlesource.com/1203750 Reviewed-by: Ben Wells <benwells@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#18} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/04d51521948290eed20beb3c30e631d70f2ed8c5/chrome/browser/extensions/bookmark_app_navigation_throttle_browsertest.cc [modify] https://crrev.com/04d51521948290eed20beb3c30e631d70f2ed8c5/chrome/browser/extensions/extension_util.cc [modify] https://crrev.com/04d51521948290eed20beb3c30e631d70f2ed8c5/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/02cd13bc870bb93f179d7f5dafea2544344efb5c commit 02cd13bc870bb93f179d7f5dafea2544344efb5c Author: Ben Wells <benwells@chromium.org> Date: Tue Sep 04 07:41:12 2018 Update chrome://apps for bookmark apps that aren't locally installed. When bookmark apps aren't locally installed, they should be dimmed out and they should have the launch type, app info and create shortcuts menus removed. This change also cleans up the logic that shows these menus and moved it to C++ where it is easier to reason about. TBR=benwells@chromium.org (cherry picked from commit 72ecaf0c5162f100ceb3697b4ca49c26ad98bdf0) Bug: 874841 Change-Id: Ia6af16a383c6f4434ac9da9e49882186c2c20015 Reviewed-on: https://chromium-review.googlesource.com/1195124 Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Commit-Queue: Ben Wells <benwells@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#587920} Reviewed-on: https://chromium-review.googlesource.com/1203751 Reviewed-by: Ben Wells <benwells@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#19} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/02cd13bc870bb93f179d7f5dafea2544344efb5c/chrome/browser/resources/ntp4/apps_page.js [modify] https://crrev.com/02cd13bc870bb93f179d7f5dafea2544344efb5c/chrome/browser/resources/ntp4/page_list_view.js [modify] https://crrev.com/02cd13bc870bb93f179d7f5dafea2544344efb5c/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/04d51521948290eed20beb3c30e631d70f2ed8c5 commit 04d51521948290eed20beb3c30e631d70f2ed8c5 Author: Ben Wells <benwells@chromium.org> Date: Tue Sep 04 07:39:50 2018 Don't treat bookmark apps that aren't locally installed as PWAs. This means if a PWA is installed on one machine and synced to another machine such that it isn't locally installed (i.e. it is synced to a non-Chrome OS machine), it won't be treated as an installed PWA. This affects features such as the navigation options available. TBR=benwells@chromium.org (cherry picked from commit e13d3169126389cf036e23f19bcf107dd7bc7652) Bug: 874841 Change-Id: I42667edbd7d9ac7dbe2a088d4b11bda422beee8b Reviewed-on: https://chromium-review.googlesource.com/1195382 Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Commit-Queue: Ben Wells <benwells@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#587874} Reviewed-on: https://chromium-review.googlesource.com/1203750 Reviewed-by: Ben Wells <benwells@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#18} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/04d51521948290eed20beb3c30e631d70f2ed8c5/chrome/browser/extensions/bookmark_app_navigation_throttle_browsertest.cc [modify] https://crrev.com/04d51521948290eed20beb3c30e631d70f2ed8c5/chrome/browser/extensions/extension_util.cc [modify] https://crrev.com/04d51521948290eed20beb3c30e631d70f2ed8c5/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/02cd13bc870bb93f179d7f5dafea2544344efb5c commit 02cd13bc870bb93f179d7f5dafea2544344efb5c Author: Ben Wells <benwells@chromium.org> Date: Tue Sep 04 07:41:12 2018 Update chrome://apps for bookmark apps that aren't locally installed. When bookmark apps aren't locally installed, they should be dimmed out and they should have the launch type, app info and create shortcuts menus removed. This change also cleans up the logic that shows these menus and moved it to C++ where it is easier to reason about. TBR=benwells@chromium.org (cherry picked from commit 72ecaf0c5162f100ceb3697b4ca49c26ad98bdf0) Bug: 874841 Change-Id: Ia6af16a383c6f4434ac9da9e49882186c2c20015 Reviewed-on: https://chromium-review.googlesource.com/1195124 Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Commit-Queue: Ben Wells <benwells@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#587920} Reviewed-on: https://chromium-review.googlesource.com/1203751 Reviewed-by: Ben Wells <benwells@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#19} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/02cd13bc870bb93f179d7f5dafea2544344efb5c/chrome/browser/resources/ntp4/apps_page.js [modify] https://crrev.com/02cd13bc870bb93f179d7f5dafea2544344efb5c/chrome/browser/resources/ntp4/page_list_view.js [modify] https://crrev.com/02cd13bc870bb93f179d7f5dafea2544344efb5c/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/04d51521948290eed20beb3c30e631d70f2ed8c5 commit 04d51521948290eed20beb3c30e631d70f2ed8c5 Author: Ben Wells <benwells@chromium.org> Date: Tue Sep 04 07:39:50 2018 Don't treat bookmark apps that aren't locally installed as PWAs. This means if a PWA is installed on one machine and synced to another machine such that it isn't locally installed (i.e. it is synced to a non-Chrome OS machine), it won't be treated as an installed PWA. This affects features such as the navigation options available. TBR=benwells@chromium.org (cherry picked from commit e13d3169126389cf036e23f19bcf107dd7bc7652) Bug: 874841 Change-Id: I42667edbd7d9ac7dbe2a088d4b11bda422beee8b Reviewed-on: https://chromium-review.googlesource.com/1195382 Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Commit-Queue: Ben Wells <benwells@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#587874} Reviewed-on: https://chromium-review.googlesource.com/1203750 Reviewed-by: Ben Wells <benwells@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#18} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/04d51521948290eed20beb3c30e631d70f2ed8c5/chrome/browser/extensions/bookmark_app_navigation_throttle_browsertest.cc [modify] https://crrev.com/04d51521948290eed20beb3c30e631d70f2ed8c5/chrome/browser/extensions/extension_util.cc [modify] https://crrev.com/04d51521948290eed20beb3c30e631d70f2ed8c5/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
,
Sep 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/02cd13bc870bb93f179d7f5dafea2544344efb5c commit 02cd13bc870bb93f179d7f5dafea2544344efb5c Author: Ben Wells <benwells@chromium.org> Date: Tue Sep 04 07:41:12 2018 Update chrome://apps for bookmark apps that aren't locally installed. When bookmark apps aren't locally installed, they should be dimmed out and they should have the launch type, app info and create shortcuts menus removed. This change also cleans up the logic that shows these menus and moved it to C++ where it is easier to reason about. TBR=benwells@chromium.org (cherry picked from commit 72ecaf0c5162f100ceb3697b4ca49c26ad98bdf0) Bug: 874841 Change-Id: Ia6af16a383c6f4434ac9da9e49882186c2c20015 Reviewed-on: https://chromium-review.googlesource.com/1195124 Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Commit-Queue: Ben Wells <benwells@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#587920} Reviewed-on: https://chromium-review.googlesource.com/1203751 Reviewed-by: Ben Wells <benwells@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#19} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/02cd13bc870bb93f179d7f5dafea2544344efb5c/chrome/browser/resources/ntp4/apps_page.js [modify] https://crrev.com/02cd13bc870bb93f179d7f5dafea2544344efb5c/chrome/browser/resources/ntp4/page_list_view.js [modify] https://crrev.com/02cd13bc870bb93f179d7f5dafea2544344efb5c/chrome/browser/ui/webui/ntp/app_launcher_handler.cc |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by benwells@chromium.org
, Aug 16