Currently we just return an empty string if the app failed to install. We should also return an error code that clients can use for metrics.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bdbdb5a2b488e887f3b76dabe58c9a3130a839bf commit bdbdb5a2b488e887f3b76dabe58c9a3130a839bf Author: Nigel Tao <nigeltao@chromium.org> Date: Thu Sep 13 01:52:50 2018 Add status codes to web app install callbacks Previously, a base::Optional<std::string app_id> was passed to the callback, with the base::Optional part indicating binary success or failure. This had some problems: - It did not distinguish between success due to a fresh install or success due to the app being previously installed. For UMA metrics, we want to distinguish these cases. - The app_id is an implementation detail, and would likely change if web apps move off extensions as per http://crbug.com/877898 After this commit, the callback no longer provides the string app_id implementation detail. Bug: 876170 Change-Id: I29a0e03ffec760bffb87bc8d45e655431d97cc21 Reviewed-on: https://chromium-review.googlesource.com/1195174 Commit-Queue: Nigel Tao <nigeltao@chromium.org> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Reviewed-by: Dominick Ng <dominickn@chromium.org> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Reviewed-by: Jeremy Klein <jlklein@chromium.org> Cr-Commit-Position: refs/heads/master@{#590890} [modify] https://crrev.com/bdbdb5a2b488e887f3b76dabe58c9a3130a839bf/chrome/browser/chromeos/multidevice_setup/android_sms_app_helper_delegate_impl.cc [modify] https://crrev.com/bdbdb5a2b488e887f3b76dabe58c9a3130a839bf/chrome/browser/chromeos/multidevice_setup/android_sms_app_helper_delegate_impl.h [modify] https://crrev.com/bdbdb5a2b488e887f3b76dabe58c9a3130a839bf/chrome/browser/web_applications/components/BUILD.gn [add] https://crrev.com/bdbdb5a2b488e887f3b76dabe58c9a3130a839bf/chrome/browser/web_applications/components/install_result_code.h [modify] https://crrev.com/bdbdb5a2b488e887f3b76dabe58c9a3130a839bf/chrome/browser/web_applications/components/pending_app_manager.h [modify] https://crrev.com/bdbdb5a2b488e887f3b76dabe58c9a3130a839bf/chrome/browser/web_applications/components/test_pending_app_manager.cc [modify] https://crrev.com/bdbdb5a2b488e887f3b76dabe58c9a3130a839bf/chrome/browser/web_applications/extensions/bookmark_app_installation_task.h [modify] https://crrev.com/bdbdb5a2b488e887f3b76dabe58c9a3130a839bf/chrome/browser/web_applications/extensions/pending_bookmark_app_manager.cc [modify] https://crrev.com/bdbdb5a2b488e887f3b76dabe58c9a3130a839bf/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_browsertest.cc [modify] https://crrev.com/bdbdb5a2b488e887f3b76dabe58c9a3130a839bf/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_unittest.cc
Fixed? InstallResultCode et al: https://cs.chromium.org/chromium/src/chrome/browser/web_applications/components/web_app_constants.h?l=29
Comment 1 by ortuno@chromium.org
, Aug 22