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

Issue 864904 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 19
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Feature


Sign in to add a comment

desktop-pwas: Introduce a queue of pending bookmark app installations

Project Member Reported by ortuno@chromium.org, Jul 18

Issue description

There are many clients that wish to install multiple WebApps based on a URL. For example, WebAppPolicyManager and in the feature InstallExternalWebApps.

These clients need a class where they can offload a set of apps to install and get notified each time an apps gets installed or fails to install.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 25

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

commit 271650d59b4b97e69ff012e01ad64c3c973e44e1
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Wed Jul 25 03:48:32 2018

desktop-pwas: Skeleton for BookmarkAppInstallationTask

Introduces the BookmarkAppInstallationTask. This class purpose is to
perform the necessary steps to install an BookmarkApp from a
WebContents or a WebApplicationInfo.

This class will have two subclasses BookmarkAppShortcutInstallation task
and BookmarkAppWebAppInstallationTask. The former is included in this
CL and installs BookmarkApp-based Shortcuts. The latter will install
BookmarkApp-based Web Apps.

Bug:  864904 

Change-Id: I5b5b9f44ff0984b0138663cb7c84abcba1062c77
Reviewed-on: https://chromium-review.googlesource.com/1131024
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577790}
[modify] https://crrev.com/271650d59b4b97e69ff012e01ad64c3c973e44e1/chrome/browser/web_applications/BUILD.gn
[modify] https://crrev.com/271650d59b4b97e69ff012e01ad64c3c973e44e1/chrome/browser/web_applications/extensions/BUILD.gn
[add] https://crrev.com/271650d59b4b97e69ff012e01ad64c3c973e44e1/chrome/browser/web_applications/extensions/bookmark_app_data_retriever.cc
[add] https://crrev.com/271650d59b4b97e69ff012e01ad64c3c973e44e1/chrome/browser/web_applications/extensions/bookmark_app_data_retriever.h
[add] https://crrev.com/271650d59b4b97e69ff012e01ad64c3c973e44e1/chrome/browser/web_applications/extensions/bookmark_app_data_retriever_unittest.cc
[add] https://crrev.com/271650d59b4b97e69ff012e01ad64c3c973e44e1/chrome/browser/web_applications/extensions/bookmark_app_installation_task.cc
[add] https://crrev.com/271650d59b4b97e69ff012e01ad64c3c973e44e1/chrome/browser/web_applications/extensions/bookmark_app_installation_task.h
[add] https://crrev.com/271650d59b4b97e69ff012e01ad64c3c973e44e1/chrome/browser/web_applications/extensions/bookmark_app_installation_task_unittest.cc
[add] https://crrev.com/271650d59b4b97e69ff012e01ad64c3c973e44e1/chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.cc
[add] https://crrev.com/271650d59b4b97e69ff012e01ad64c3c973e44e1/chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.h
[modify] https://crrev.com/271650d59b4b97e69ff012e01ad64c3c973e44e1/content/public/test/web_contents_tester.h
[modify] https://crrev.com/271650d59b4b97e69ff012e01ad64c3c973e44e1/content/test/test_web_contents.cc
[modify] https://crrev.com/271650d59b4b97e69ff012e01ad64c3c973e44e1/content/test/test_web_contents.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 25

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

commit 9182ff2910d100e84671a3f7cf59d98e457289c7
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Wed Jul 25 06:17:07 2018

desktop-pwas: Generate icons as part of the BookmarkApp installation task

Adds code to BookmarkAppShortcutInstallationTask to get the
necessary icons from the DataRetriever. And Adds code to
BookmarkAppDataRetriever to generate all icons necessary to install
a Bookmark App. In the future, BookmarkAppDataRetriever will take a list
of icon URLs to download, download the icons, resize them and generate
icons for any missing sizes. But for now we just generate all icons.

Bug:  864904 
Change-Id: Ia90e67174e89fa6e92b54bee812ab337fed821c6
Reviewed-on: https://chromium-review.googlesource.com/1147899
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577812}
[modify] https://crrev.com/9182ff2910d100e84671a3f7cf59d98e457289c7/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/9182ff2910d100e84671a3f7cf59d98e457289c7/chrome/browser/extensions/bookmark_app_helper.h
[modify] https://crrev.com/9182ff2910d100e84671a3f7cf59d98e457289c7/chrome/browser/web_applications/extensions/bookmark_app_data_retriever.cc
[modify] https://crrev.com/9182ff2910d100e84671a3f7cf59d98e457289c7/chrome/browser/web_applications/extensions/bookmark_app_data_retriever.h
[modify] https://crrev.com/9182ff2910d100e84671a3f7cf59d98e457289c7/chrome/browser/web_applications/extensions/bookmark_app_data_retriever_unittest.cc
[modify] https://crrev.com/9182ff2910d100e84671a3f7cf59d98e457289c7/chrome/browser/web_applications/extensions/bookmark_app_installation_task_unittest.cc
[modify] https://crrev.com/9182ff2910d100e84671a3f7cf59d98e457289c7/chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.cc
[modify] https://crrev.com/9182ff2910d100e84671a3f7cf59d98e457289c7/chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 30

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

commit 6d158c31f47ac0bc300cdf6ef5e6da19b0845856
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Mon Jul 30 06:24:40 2018

desktop-pwas: Wrap WebApplicationInfo in a unique_ptr

WebApplicationInfo is not moveable so wwhen calling std::move() on it,
we make a copy. To avoid this, wrap WebApplicationInfo in a unique_ptr.

Bug:  864904 
Change-Id: I3568101b8ee209786b18127e515b694729aa2ed2
Reviewed-on: https://chromium-review.googlesource.com/1154733
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578983}
[modify] https://crrev.com/6d158c31f47ac0bc300cdf6ef5e6da19b0845856/chrome/browser/web_applications/extensions/bookmark_app_data_retriever.cc
[modify] https://crrev.com/6d158c31f47ac0bc300cdf6ef5e6da19b0845856/chrome/browser/web_applications/extensions/bookmark_app_data_retriever.h
[modify] https://crrev.com/6d158c31f47ac0bc300cdf6ef5e6da19b0845856/chrome/browser/web_applications/extensions/bookmark_app_data_retriever_unittest.cc
[modify] https://crrev.com/6d158c31f47ac0bc300cdf6ef5e6da19b0845856/chrome/browser/web_applications/extensions/bookmark_app_installation_task_unittest.cc
[modify] https://crrev.com/6d158c31f47ac0bc300cdf6ef5e6da19b0845856/chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.cc
[modify] https://crrev.com/6d158c31f47ac0bc300cdf6ef5e6da19b0845856/chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 31

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

commit 4f86bf271c69c0aa18577c0ce982d1b345168328
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Tue Jul 31 02:09:03 2018

desktop-pwas: Move PendingAppManager to be owned by WebAppProvider

Moves the ownership of PendingAppManager up to WebAppProvider.
PendingAppManager will be used by any client that wishes to install a
WebApp. For now this is only WebAppPolicyManager and
ScanForExternalApps.

Bug:  864904 
Change-Id: I0599c896accdd74ceabfc16bf403d4eec501c97e
Reviewed-on: https://chromium-review.googlesource.com/1154744
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579277}
[modify] https://crrev.com/4f86bf271c69c0aa18577c0ce982d1b345168328/chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager.cc
[modify] https://crrev.com/4f86bf271c69c0aa18577c0ce982d1b345168328/chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager.h
[modify] https://crrev.com/4f86bf271c69c0aa18577c0ce982d1b345168328/chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager_unittest.cc
[modify] https://crrev.com/4f86bf271c69c0aa18577c0ce982d1b345168328/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h
[modify] https://crrev.com/4f86bf271c69c0aa18577c0ce982d1b345168328/chrome/browser/web_applications/web_app_provider.cc
[modify] https://crrev.com/4f86bf271c69c0aa18577c0ce982d1b345168328/chrome/browser/web_applications/web_app_provider.h

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 3

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

commit 82800b5be0fc5443b270ba859f3ceceb9c73ab12
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Fri Aug 03 07:54:10 2018

desktop-pwas: Add extension creation subtask to installation task

Introduces a BookmarkAppInstaller class whose job is to create
a Bookmark App with certain characteristics e.g. name, start url,
launch container, etc.

The BookmarkAppInstaller class uses CrxInstaller to actually
create and install the BookmarkApp. Currently the installer only
takes a WebApplicationInfo but in the future it'll support more
options, for example, if the app will be synced, what container
will the app launch in, etc.

Marks BookmarkAppInstallerTest as a friend in CrxInstaller so that
we can create a fake CrxInstaller that fails to install the app.

Bug:  864904 
Change-Id: I105d1e93160f2527ee6cb5c4951fab885a893392
Reviewed-on: https://chromium-review.googlesource.com/1152642
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580471}
[modify] https://crrev.com/82800b5be0fc5443b270ba859f3ceceb9c73ab12/chrome/browser/extensions/crx_installer.h
[modify] https://crrev.com/82800b5be0fc5443b270ba859f3ceceb9c73ab12/chrome/browser/web_applications/extensions/BUILD.gn
[modify] https://crrev.com/82800b5be0fc5443b270ba859f3ceceb9c73ab12/chrome/browser/web_applications/extensions/bookmark_app_installation_task.cc
[modify] https://crrev.com/82800b5be0fc5443b270ba859f3ceceb9c73ab12/chrome/browser/web_applications/extensions/bookmark_app_installation_task.h
[modify] https://crrev.com/82800b5be0fc5443b270ba859f3ceceb9c73ab12/chrome/browser/web_applications/extensions/bookmark_app_installation_task_unittest.cc
[add] https://crrev.com/82800b5be0fc5443b270ba859f3ceceb9c73ab12/chrome/browser/web_applications/extensions/bookmark_app_installer.cc
[add] https://crrev.com/82800b5be0fc5443b270ba859f3ceceb9c73ab12/chrome/browser/web_applications/extensions/bookmark_app_installer.h
[add] https://crrev.com/82800b5be0fc5443b270ba859f3ceceb9c73ab12/chrome/browser/web_applications/extensions/bookmark_app_installer_unittest.cc
[modify] https://crrev.com/82800b5be0fc5443b270ba859f3ceceb9c73ab12/chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.cc
[modify] https://crrev.com/82800b5be0fc5443b270ba859f3ceceb9c73ab12/chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.h

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 6

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

commit 8ea2f1e48b5f0162b42e5f2440ddffe6fb49a1ee
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Mon Aug 06 01:13:05 2018

desktop-pwas: Add a Install function to PendingAppManager

Adds a function to start the installation of an app and implements it
for PendingBookmarkAppManager. If there is an app being installed the
installation is delayed until the current installation finishes.

Bug:  864904 
Change-Id: I2397b243dd8065eb67edd055536305e8da0201a1
Reviewed-on: https://chromium-review.googlesource.com/1157948
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580789}
[modify] https://crrev.com/8ea2f1e48b5f0162b42e5f2440ddffe6fb49a1ee/chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager_unittest.cc
[modify] https://crrev.com/8ea2f1e48b5f0162b42e5f2440ddffe6fb49a1ee/chrome/browser/web_applications/components/pending_app_manager.h
[modify] https://crrev.com/8ea2f1e48b5f0162b42e5f2440ddffe6fb49a1ee/chrome/browser/web_applications/extensions/BUILD.gn
[modify] https://crrev.com/8ea2f1e48b5f0162b42e5f2440ddffe6fb49a1ee/chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.h
[modify] https://crrev.com/8ea2f1e48b5f0162b42e5f2440ddffe6fb49a1ee/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc
[modify] https://crrev.com/8ea2f1e48b5f0162b42e5f2440ddffe6fb49a1ee/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h
[add] https://crrev.com/8ea2f1e48b5f0162b42e5f2440ddffe6fb49a1ee/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc
[modify] https://crrev.com/8ea2f1e48b5f0162b42e5f2440ddffe6fb49a1ee/chrome/browser/web_applications/web_app_provider.cc
[modify] https://crrev.com/8ea2f1e48b5f0162b42e5f2440ddffe6fb49a1ee/chrome/browser/web_applications/web_app_provider.h
[modify] https://crrev.com/8ea2f1e48b5f0162b42e5f2440ddffe6fb49a1ee/chrome/browser/web_applications/web_app_provider_factory.cc
[modify] https://crrev.com/8ea2f1e48b5f0162b42e5f2440ddffe6fb49a1ee/content/public/test/web_contents_tester.h
[modify] https://crrev.com/8ea2f1e48b5f0162b42e5f2440ddffe6fb49a1ee/content/test/test_web_contents.cc
[modify] https://crrev.com/8ea2f1e48b5f0162b42e5f2440ddffe6fb49a1ee/content/test/test_web_contents.h

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 8

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

commit 5e5b1bb80293aa99f2236f85f54ec89cde46a8c2
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Wed Aug 08 23:08:56 2018

desktop-pwas: Change InstallationTask to return an app id when the task is done

Will be used by clients to associated their installation request to the
installed app.

Bug:  864904 
Change-Id: Ib439b669340373e8db3822a2c149f6ea2041d631
Reviewed-on: https://chromium-review.googlesource.com/1156191
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581720}
[modify] https://crrev.com/5e5b1bb80293aa99f2236f85f54ec89cde46a8c2/chrome/browser/web_applications/components/pending_app_manager.h
[modify] https://crrev.com/5e5b1bb80293aa99f2236f85f54ec89cde46a8c2/chrome/browser/web_applications/extensions/bookmark_app_installation_task.cc
[modify] https://crrev.com/5e5b1bb80293aa99f2236f85f54ec89cde46a8c2/chrome/browser/web_applications/extensions/bookmark_app_installation_task.h
[modify] https://crrev.com/5e5b1bb80293aa99f2236f85f54ec89cde46a8c2/chrome/browser/web_applications/extensions/bookmark_app_installation_task_unittest.cc
[modify] https://crrev.com/5e5b1bb80293aa99f2236f85f54ec89cde46a8c2/chrome/browser/web_applications/extensions/bookmark_app_installer.cc
[modify] https://crrev.com/5e5b1bb80293aa99f2236f85f54ec89cde46a8c2/chrome/browser/web_applications/extensions/bookmark_app_installer.h
[modify] https://crrev.com/5e5b1bb80293aa99f2236f85f54ec89cde46a8c2/chrome/browser/web_applications/extensions/bookmark_app_installer_unittest.cc
[modify] https://crrev.com/5e5b1bb80293aa99f2236f85f54ec89cde46a8c2/chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.cc
[modify] https://crrev.com/5e5b1bb80293aa99f2236f85f54ec89cde46a8c2/chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.h
[modify] https://crrev.com/5e5b1bb80293aa99f2236f85f54ec89cde46a8c2/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc
[modify] https://crrev.com/5e5b1bb80293aa99f2236f85f54ec89cde46a8c2/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 14

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

commit 77012d26bc97b208a54c75f0e5ebcb93cad14da0
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Tue Aug 14 01:20:30 2018

desktop-pwas: Add queue to PendingBookmarkAppManager

Adds a queue to process installations to PendingBookmarkAppManager.

Multiple calls to Install() will result in installations being
added to the queue. Once an installation finishes, the next one
will start.

Bug:  864904 
Change-Id: I063c1dd16760183a1fbbde64ed7b43bf89078dfe
Reviewed-on: https://chromium-review.googlesource.com/1163403
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582781}
[modify] https://crrev.com/77012d26bc97b208a54c75f0e5ebcb93cad14da0/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc
[modify] https://crrev.com/77012d26bc97b208a54c75f0e5ebcb93cad14da0/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h
[modify] https://crrev.com/77012d26bc97b208a54c75f0e5ebcb93cad14da0/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 15

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

commit 60eb76e65c24e823733e4bb55b42cc4e390ef20f
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Wed Aug 15 06:44:09 2018

desktop-pwas: Change InstallCallback to return a GURL in addition to the app id

ProcessAppOperations will need to include the URL of the app that was
installed. To avoid having two distinct callbacks, this CL changes
InstallCallback to also return the url.

Bug:  864904 
Change-Id: Ie296a90b4588c0d301c9a03d487433b9d56832b1
Reviewed-on: https://chromium-review.googlesource.com/1174589
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583181}
[modify] https://crrev.com/60eb76e65c24e823733e4bb55b42cc4e390ef20f/chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager_unittest.cc
[modify] https://crrev.com/60eb76e65c24e823733e4bb55b42cc4e390ef20f/chrome/browser/web_applications/components/pending_app_manager.h
[modify] https://crrev.com/60eb76e65c24e823733e4bb55b42cc4e390ef20f/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc
[modify] https://crrev.com/60eb76e65c24e823733e4bb55b42cc4e390ef20f/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h
[modify] https://crrev.com/60eb76e65c24e823733e4bb55b42cc4e390ef20f/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 16

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

commit 0383ff94923c1955377926a7a06671a403ab1636
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Thu Aug 16 02:30:14 2018

desktop-pwas: Minor PendingBookmarkAppManager unittest cleanup

Adds a helper method to initialize a PendingBookmarkAppManager with
test factories.

Bug:  864904 
Change-Id: Ibdaaa1d43455f19979db3cd29429844b8c661598
Reviewed-on: https://chromium-review.googlesource.com/1176881
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583509}
[modify] https://crrev.com/0383ff94923c1955377926a7a06671a403ab1636/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 16

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

commit 8c211c8bbcad26607db54d0793f1848452c8ab25
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Thu Aug 16 05:34:31 2018

desktop-pwas: Implement InstallApps to queue multiple install requests

PendingAppManager::InstallApps allows clients to queue the installation
of multiple apps. The installation requests are put at the back of the
queue and have lower priority than requests done through Install().

Bug:  864904 

Change-Id: I079ad6bdc60cd972f3204bee4c575a563377fd2c
Reviewed-on: https://chromium-review.googlesource.com/1173935
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583553}
[modify] https://crrev.com/8c211c8bbcad26607db54d0793f1848452c8ab25/chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager.cc
[modify] https://crrev.com/8c211c8bbcad26607db54d0793f1848452c8ab25/chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager_unittest.cc
[modify] https://crrev.com/8c211c8bbcad26607db54d0793f1848452c8ab25/chrome/browser/web_applications/components/pending_app_manager.h
[modify] https://crrev.com/8c211c8bbcad26607db54d0793f1848452c8ab25/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc
[modify] https://crrev.com/8c211c8bbcad26607db54d0793f1848452c8ab25/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h
[modify] https://crrev.com/8c211c8bbcad26607db54d0793f1848452c8ab25/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc
[modify] https://crrev.com/8c211c8bbcad26607db54d0793f1848452c8ab25/chrome/browser/web_applications/web_app_provider.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 20

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

commit 8a1d59b03df79a85aed46778539bd91299b81cc2
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Mon Aug 20 06:22:20 2018

desktop-pwas: Change BookmarkAppInstallationTask to use a BookmarkAppHelper

Use BookmarkAppHelper to perform all necessary steps to install a
Bookmark App based on a WebContents.

Bug:  864904 

Change-Id: I3c243714236caf2b93c93adfc8c62e5ee0e69ae9
Reviewed-on: https://chromium-review.googlesource.com/1163417
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584372}
[modify] https://crrev.com/8a1d59b03df79a85aed46778539bd91299b81cc2/chrome/browser/web_applications/extensions/bookmark_app_installation_task.cc
[modify] https://crrev.com/8a1d59b03df79a85aed46778539bd91299b81cc2/chrome/browser/web_applications/extensions/bookmark_app_installation_task.h
[modify] https://crrev.com/8a1d59b03df79a85aed46778539bd91299b81cc2/chrome/browser/web_applications/extensions/bookmark_app_installation_task_unittest.cc
[modify] https://crrev.com/8a1d59b03df79a85aed46778539bd91299b81cc2/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc
[add] https://crrev.com/8a1d59b03df79a85aed46778539bd91299b81cc2/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_browsertest.cc
[modify] https://crrev.com/8a1d59b03df79a85aed46778539bd91299b81cc2/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc
[modify] https://crrev.com/8a1d59b03df79a85aed46778539bd91299b81cc2/chrome/test/BUILD.gn

Blockedon: 876169
Blockedon: 876170
Blockedon: 876172
Blockedon: 876173
Blockedon: 876174
Blockedon: 876175
Blockedon: 876176
Blockedon: 876182
Blockedon: 876577
Blockedon: 877398
Project Member

Comment 23 by bugdroid1@chromium.org, Aug 28

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

commit 1b796489950a516389a984c26cde0d01522d75fa
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Tue Aug 28 00:00:42 2018

desktop-pwas: Don't check if the app is getting installed already

Checking if the app is getting installed already leads to inconsistent
behavior. For example if two calls to install the same app arrive,
the result will depend on whether or not the installation succeeded in
between calls. If the first call finishes before the second call
arrives, then we'll send two success events. If the first call is in
progress when the second call arrives, then we'll send one failure
event for the second call and a success event for the first call once
it finishes.

To fix this inconsistency, this CL removes the checks in Install().
This will cause same-app installation requests to be queued just like
any other installation request.

Bug:  864904 
Change-Id: Ic6be471fba4e55f476c6aebe398fb3eadd100d82
Reviewed-on: https://chromium-review.googlesource.com/1189520
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586507}
[modify] https://crrev.com/1b796489950a516389a984c26cde0d01522d75fa/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc
[modify] https://crrev.com/1b796489950a516389a984c26cde0d01522d75fa/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Aug 28

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

commit f14711f8118a2de62a5dead078cc089dfbe09108
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Tue Aug 28 02:53:01 2018

desktop-pwas: Return a base::Optional rather than a std::string after installation

An empty string to indicate that the installation fails is easy to miss.
Changes callbacks to return a base::Optional<> to make it more obvious
what is returned when the installation fails.

TBR=khorimoto@chromium.org

Bug:  864904 
Change-Id: I38eb2050df95a7ade1c6688e209af5eb47ba1a4c
Reviewed-on: https://chromium-review.googlesource.com/1189519
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586557}
[modify] https://crrev.com/f14711f8118a2de62a5dead078cc089dfbe09108/chrome/browser/chromeos/multidevice_setup/android_sms_app_helper_delegate_impl.cc
[modify] https://crrev.com/f14711f8118a2de62a5dead078cc089dfbe09108/chrome/browser/chromeos/multidevice_setup/android_sms_app_helper_delegate_impl.h
[modify] https://crrev.com/f14711f8118a2de62a5dead078cc089dfbe09108/chrome/browser/web_applications/components/pending_app_manager.h
[modify] https://crrev.com/f14711f8118a2de62a5dead078cc089dfbe09108/chrome/browser/web_applications/extensions/bookmark_app_installation_task.cc
[modify] https://crrev.com/f14711f8118a2de62a5dead078cc089dfbe09108/chrome/browser/web_applications/extensions/bookmark_app_installation_task.h
[modify] https://crrev.com/f14711f8118a2de62a5dead078cc089dfbe09108/chrome/browser/web_applications/extensions/bookmark_app_installation_task_unittest.cc
[modify] https://crrev.com/f14711f8118a2de62a5dead078cc089dfbe09108/chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.cc
[modify] https://crrev.com/f14711f8118a2de62a5dead078cc089dfbe09108/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc
[modify] https://crrev.com/f14711f8118a2de62a5dead078cc089dfbe09108/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.h
[modify] https://crrev.com/f14711f8118a2de62a5dead078cc089dfbe09108/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_browsertest.cc
[modify] https://crrev.com/f14711f8118a2de62a5dead078cc089dfbe09108/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc

Blockedon: 878663
Is it finished?
Status: Fixed (was: Started)
Yeah pretty much. I think we could add more logging and be more specific with errors, but the main part is pretty much done.

Sign in to add a comment