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

Issue 898074 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 29
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Installation of default web apps should fail if there is no manifest.

Project Member Reported by benwells@chromium.org, Oct 23

Issue description

Default web apps should always have good metadata, so if they do not the installation should fail.

This change is required to prevent a default app with a region restricted manifest being installed in a poor way for users in the wrong region.
 
Components: UI>Browser>WebAppInstalls
Cc: -dominickn@google.com dominickn@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 25

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

commit 2f34991d490fe3788cb12aaefc42cdab73711f9a
Author: Ben Wells <benwells@chromium.org>
Date: Thu Oct 25 03:50:23 2018

Require a manifest for default installed web apps.

This ensures good metadata is present for pre installed apps.

Bug:  898074 
Change-Id: Idb529dc65bc9c804599990f63c19146720c6f327
Reviewed-on: https://chromium-review.googlesource.com/c/1295635
Commit-Queue: Ben Wells <benwells@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602602}
[modify] https://crrev.com/2f34991d490fe3788cb12aaefc42cdab73711f9a/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/2f34991d490fe3788cb12aaefc42cdab73711f9a/chrome/browser/extensions/bookmark_app_helper.h
[modify] https://crrev.com/2f34991d490fe3788cb12aaefc42cdab73711f9a/chrome/browser/web_applications/bookmark_apps/external_web_apps.cc
[modify] https://crrev.com/2f34991d490fe3788cb12aaefc42cdab73711f9a/chrome/browser/web_applications/bookmark_apps/external_web_apps_unittest.cc
[modify] https://crrev.com/2f34991d490fe3788cb12aaefc42cdab73711f9a/chrome/browser/web_applications/components/pending_app_manager.cc
[modify] https://crrev.com/2f34991d490fe3788cb12aaefc42cdab73711f9a/chrome/browser/web_applications/components/pending_app_manager.h
[modify] https://crrev.com/2f34991d490fe3788cb12aaefc42cdab73711f9a/chrome/browser/web_applications/extensions/bookmark_app_installation_task.cc
[modify] https://crrev.com/2f34991d490fe3788cb12aaefc42cdab73711f9a/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_browsertest.cc

Labels: Merge-Request-71
Merge people: would be great to get this merged before the next dev channel, behaiour is sub-optimal without it.
Pls apply appropriate OSs label.


Labels: OS-Chrome
(sorry about that!)
This change impacts a number of areas.  Can you add context regarding testing on ToT/M72 or otherwise prior to a merge?  I'd like to understand the merge risk.  Thanks.

This change touches code related to installation of web apps. The code it touches is involved in the manual installation of web apps as well as default / automatic, however due to the defaults of the new value it should not affect manual installation.

On ToT I have tested it has the desired effect with default installed apps, as well as no effect on manually installed web apps (with or without a manifest).
Project Member

Comment 10 by sheriffbot@chromium.org, Oct 26

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-71 Merge-Approved-71
Thanks for #9
Approving merge to M71 Chrome OS.
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 29

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fd572c9357cf84cd9017eb30d32aa45804c0d461

commit fd572c9357cf84cd9017eb30d32aa45804c0d461
Author: Ben Wells <benwells@chromium.org>
Date: Mon Oct 29 04:55:48 2018

Require a manifest for default installed web apps.

This ensures good metadata is present for pre installed apps.

TBR=benwells@chromium.org

(cherry picked from commit 2f34991d490fe3788cb12aaefc42cdab73711f9a)

Bug:  898074 
Change-Id: Idb529dc65bc9c804599990f63c19146720c6f327
Reviewed-on: https://chromium-review.googlesource.com/c/1295635
Commit-Queue: Ben Wells <benwells@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602602}
Reviewed-on: https://chromium-review.googlesource.com/c/1304039
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#363}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/fd572c9357cf84cd9017eb30d32aa45804c0d461/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/fd572c9357cf84cd9017eb30d32aa45804c0d461/chrome/browser/extensions/bookmark_app_helper.h
[modify] https://crrev.com/fd572c9357cf84cd9017eb30d32aa45804c0d461/chrome/browser/web_applications/bookmark_apps/external_web_apps.cc
[modify] https://crrev.com/fd572c9357cf84cd9017eb30d32aa45804c0d461/chrome/browser/web_applications/bookmark_apps/external_web_apps_unittest.cc
[modify] https://crrev.com/fd572c9357cf84cd9017eb30d32aa45804c0d461/chrome/browser/web_applications/components/pending_app_manager.cc
[modify] https://crrev.com/fd572c9357cf84cd9017eb30d32aa45804c0d461/chrome/browser/web_applications/components/pending_app_manager.h
[modify] https://crrev.com/fd572c9357cf84cd9017eb30d32aa45804c0d461/chrome/browser/web_applications/extensions/bookmark_app_installation_task.cc
[modify] https://crrev.com/fd572c9357cf84cd9017eb30d32aa45804c0d461/chrome/browser/web_applications/extensions/pending_bookmark_app_manager_browsertest.cc

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/fd572c9357cf84cd9017eb30d32aa45804c0d461

Commit: fd572c9357cf84cd9017eb30d32aa45804c0d461
Author: benwells@chromium.org
Commiter: benwells@chromium.org
Date: 2018-10-29 04:55:48 +0000 UTC

Require a manifest for default installed web apps.

This ensures good metadata is present for pre installed apps.

TBR=benwells@chromium.org

(cherry picked from commit 2f34991d490fe3788cb12aaefc42cdab73711f9a)

Bug:  898074 
Change-Id: Idb529dc65bc9c804599990f63c19146720c6f327
Reviewed-on: https://chromium-review.googlesource.com/c/1295635
Commit-Queue: Ben Wells <benwells@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602602}
Reviewed-on: https://chromium-review.googlesource.com/c/1304039
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#363}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Status: Fixed (was: Started)

Sign in to add a comment