New issue
Advanced search Search tips

Issue 860581 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Feature

Blocked on:
issue 862049

Blocking:
issue 877898



Sign in to add a comment

Create "OS-shortcut Manager" component

Project Member Reported by loyso@chromium.org, Jul 6

Issue description

A Shortcut Manager helps with adding shortcuts to different platforms.

Extract a generic App Shortcut manager which the Extensions System uses.
 
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 12

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

commit d538ba3e10d2498292aa429cef90d8b6c18473cd
Author: Alexey Baskakov <loyso@chromium.org>
Date: Thu Jul 12 01:36:47 2018

WebApp: Extract web_applications source sets in GN.

This CL reflects the existing dependencies and dependency
cycles we have.

Also, we want to establish the following folders in web_applications/:
1) components/
Any independent components which are shared between extensions/
and web_applications/

2) extensions/
A code which is thematically part of chrome/browser/extensions/
E.g. an upcoming BookmarkAppInstaller will be used by extensions.

3) bookmark_apps/
Any extension-based high-level helpers which are used by chrome/browser/

Note that web_applications/policy content will be moved to
web_applications/bookmark_apps/policy/

Bug: 860581
Change-Id: Ib89f0e327759cfab050caceae5818c8e72f40f08
Reviewed-on: https://chromium-review.googlesource.com/1128665
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574448}
[modify] https://crrev.com/d538ba3e10d2498292aa429cef90d8b6c18473cd/chrome/browser/BUILD.gn
[modify] https://crrev.com/d538ba3e10d2498292aa429cef90d8b6c18473cd/chrome/browser/extensions/BUILD.gn
[modify] https://crrev.com/d538ba3e10d2498292aa429cef90d8b6c18473cd/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/d538ba3e10d2498292aa429cef90d8b6c18473cd/chrome/browser/web_applications/BUILD.gn
[add] https://crrev.com/d538ba3e10d2498292aa429cef90d8b6c18473cd/chrome/browser/web_applications/bookmark_apps/BUILD.gn
[add] https://crrev.com/d538ba3e10d2498292aa429cef90d8b6c18473cd/chrome/browser/web_applications/components/BUILD.gn
[add] https://crrev.com/d538ba3e10d2498292aa429cef90d8b6c18473cd/chrome/browser/web_applications/extensions/BUILD.gn
[add] https://crrev.com/d538ba3e10d2498292aa429cef90d8b6c18473cd/chrome/browser/web_applications/policy/BUILD.gn
[modify] https://crrev.com/d538ba3e10d2498292aa429cef90d8b6c18473cd/chrome/test/BUILD.gn

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 13

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

commit cbade2ed6ea962f17c0c550083016ffe97169e7f
Author: Alexey Baskakov <loyso@chromium.org>
Date: Fri Jul 13 01:18:37 2018

WebApp: Remove chrome/ dependency in web_app.h

1) WebApplicationInfo and web_app.h are unrelated.
2) shell_integration.h is used only in win.

Drive by: Remove some unused includes.

Bug: 860581
Change-Id: Ifd0cec57dd0a54d81487f50fed0de44b487076b6
Reviewed-on: https://chromium-review.googlesource.com/1134639
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574799}
[modify] https://crrev.com/cbade2ed6ea962f17c0c550083016ffe97169e7f/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/cbade2ed6ea962f17c0c550083016ffe97169e7f/chrome/browser/extensions/bookmark_app_navigation_throttle.cc
[modify] https://crrev.com/cbade2ed6ea962f17c0c550083016ffe97169e7f/chrome/browser/extensions/crx_installer.cc
[modify] https://crrev.com/cbade2ed6ea962f17c0c550083016ffe97169e7f/chrome/browser/ui/browser_navigator_browsertest.cc
[modify] https://crrev.com/cbade2ed6ea962f17c0c550083016ffe97169e7f/chrome/browser/ui/extensions/hosted_app_browsertest.cc
[modify] https://crrev.com/cbade2ed6ea962f17c0c550083016ffe97169e7f/chrome/browser/web_applications/web_app.h
[modify] https://crrev.com/cbade2ed6ea962f17c0c550083016ffe97169e7f/chrome/browser/web_applications/web_app_win.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 13

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

commit ccad80579b6755c8cb01838031ebf5fa15e9c37d
Author: Alexey Baskakov <loyso@chromium.org>
Date: Fri Jul 13 05:43:31 2018

WebApp: Move ShortcutInfo static functions out to internals

We plan to re-group and move web_app.h code to
components/, extensions/, bookmark_apps/
buckets later.

Modernize: Use OnceCallback where possible.

Bug: 860581
Change-Id: Iae55d9d0a665a7cb1df05cba15491fe9c82b819c
Reviewed-on: https://chromium-review.googlesource.com/1136162
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574845}
[modify] https://crrev.com/ccad80579b6755c8cb01838031ebf5fa15e9c37d/chrome/browser/apps/platform_apps/shortcut_manager.cc
[modify] https://crrev.com/ccad80579b6755c8cb01838031ebf5fa15e9c37d/chrome/browser/web_applications/web_app.cc
[modify] https://crrev.com/ccad80579b6755c8cb01838031ebf5fa15e9c37d/chrome/browser/web_applications/web_app.h
[modify] https://crrev.com/ccad80579b6755c8cb01838031ebf5fa15e9c37d/chrome/browser/web_applications/web_app_chromeos.cc
[modify] https://crrev.com/ccad80579b6755c8cb01838031ebf5fa15e9c37d/chrome/browser/web_applications/web_app_linux.cc
[modify] https://crrev.com/ccad80579b6755c8cb01838031ebf5fa15e9c37d/chrome/browser/web_applications/web_app_mac.mm
[modify] https://crrev.com/ccad80579b6755c8cb01838031ebf5fa15e9c37d/chrome/browser/web_applications/web_app_win.cc

Blockedon: 862049
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 30

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

commit 243d733508a4035c05d13774267028a06b6d7a83
Author: Alexey Baskakov <loyso@chromium.org>
Date: Mon Jul 30 06:05:09 2018

WebApp: Rename components_unit_tests to unit_tests.

Bug: 860581
Change-Id: Ice0a693c6a8a4a247d9cd191967e0f88af2d4bf4
Reviewed-on: https://chromium-review.googlesource.com/1154735
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578982}
[modify] https://crrev.com/243d733508a4035c05d13774267028a06b6d7a83/chrome/browser/web_applications/BUILD.gn
[modify] https://crrev.com/243d733508a4035c05d13774267028a06b6d7a83/chrome/browser/web_applications/components/BUILD.gn

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 31

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

commit 51e63525b464d10ea1a14875524e6e321b82d365
Author: Alexey Baskakov <loyso@chromium.org>
Date: Tue Jul 31 05:49:41 2018

WebApp: Add web_app_group to share configs in web_application/ build targets.

We need to enforce exit_time_destructors compiler flag warning everywhere.

Bug: 860581
Change-Id: I7cbb2ef3ba5c71a69233bc8bd50725786b6ea0b1
Reviewed-on: https://chromium-review.googlesource.com/1155276
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579323}
[modify] https://crrev.com/51e63525b464d10ea1a14875524e6e321b82d365/chrome/browser/web_applications/BUILD.gn
[modify] https://crrev.com/51e63525b464d10ea1a14875524e6e321b82d365/chrome/browser/web_applications/bookmark_apps/BUILD.gn
[modify] https://crrev.com/51e63525b464d10ea1a14875524e6e321b82d365/chrome/browser/web_applications/components/BUILD.gn
[modify] https://crrev.com/51e63525b464d10ea1a14875524e6e321b82d365/chrome/browser/web_applications/extensions/BUILD.gn

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 2

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

commit 396edb9422568dc985f7542847b5169b0a526fe4
Author: Alexey Baskakov <loyso@chromium.org>
Date: Thu Aug 02 05:11:42 2018

WebApp: Redirect Sync prefs via WebAppProvider.

We want to erase web_app_extension_helpers.h in favor of
web_app_helpers.h.

A plan:
1) Make WebAppProvider a single entry point for all external web_appplications/
clients.
2) Introduce enable_web_app_off_extensions command line switch.
3) Create new App Sync Manager component to utilize upcoming
Unified Sync and Storage (USS) system.
4) Be able to switch between old extension-based bookmark_apps/ and
new off-extensions bookmark_apps2/ implementations inside the WebAppProvider.

TBR=sky@chromium.org

Bug: 860581
Change-Id: Iab19d936fbcf39c011fc1eff1e29125b5ddb5e16
Reviewed-on: https://chromium-review.googlesource.com/1159925
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580074}
[modify] https://crrev.com/396edb9422568dc985f7542847b5169b0a526fe4/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/396edb9422568dc985f7542847b5169b0a526fe4/chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager.cc
[modify] https://crrev.com/396edb9422568dc985f7542847b5169b0a526fe4/chrome/browser/web_applications/bookmark_apps/policy/web_app_policy_manager.h
[modify] https://crrev.com/396edb9422568dc985f7542847b5169b0a526fe4/chrome/browser/web_applications/extensions/web_app_extension_helpers.cc
[modify] https://crrev.com/396edb9422568dc985f7542847b5169b0a526fe4/chrome/browser/web_applications/extensions/web_app_extension_helpers.h
[modify] https://crrev.com/396edb9422568dc985f7542847b5169b0a526fe4/chrome/browser/web_applications/web_app_provider.cc
[modify] https://crrev.com/396edb9422568dc985f7542847b5169b0a526fe4/chrome/browser/web_applications/web_app_provider.h

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 3

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 6

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

commit 2ce2ffc84e03b4e03e019ae89d536e2126f1372d
Author: Alexey Baskakov <loyso@chromium.org>
Date: Mon Aug 06 02:57:15 2018

WebApp: Rename helper functions in web_app_extension_helpers.h

Erase web_app_extension_helpers.h in favor of web_app_helpers.h

Rename legacy helpers.

TBR=lazyboy@chromium.org

Bug: 860581
Change-Id: I67799d5d8ef79ce55e3d8f8b5acb51c59afc662b
Reviewed-on: https://chromium-review.googlesource.com/1161707
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580792}
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/app_controller_mac.mm
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/extensions/api/tabs/tabs_api.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/extensions/bookmark_app_navigation_throttle_browsertest.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/extensions/bookmark_app_navigation_throttle_utils.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/extensions/browsertest_util.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/extensions/tab_helper.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/renderer_context_menu/render_view_context_menu.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/shell_integration_win.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/shell_integration_win_unittest.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/ash/launcher/browser_status_monitor.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/ash/launcher/launcher_controller_helper.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/browser.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/cocoa/apps/quit_with_apps_controller_mac.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/extensions/application_launch.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/extensions/hosted_app_browser_controller.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/extensions/hosted_app_browsertest.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/views/apps/chrome_native_app_window_views_win.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/ui/views/frame/browser_window_property_manager_win.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/web_applications/extensions/BUILD.gn
[delete] https://crrev.com/828096ff88cd17a2d3681940f3550fa105d22943/chrome/browser/web_applications/extensions/web_app_extension_helpers.cc
[delete] https://crrev.com/828096ff88cd17a2d3681940f3550fa105d22943/chrome/browser/web_applications/extensions/web_app_extension_helpers.h
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/web_applications/extensions/web_app_extension_shortcut.cc
[modify] https://crrev.com/2ce2ffc84e03b4e03e019ae89d536e2126f1372d/chrome/browser/web_applications/extensions/web_app_extension_shortcut_mac.mm

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 9

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

commit c829901563d1d0f8c2316b16f73fc2ac256ce2ae
Author: Alexey Baskakov <loyso@chromium.org>
Date: Thu Aug 09 08:41:14 2018

WebApp: Extract BookmarkAppHelper's gfx stuff into a separate file.

Extract all the low-level skia/gfx/color_util stuff as
ResizeIconsAndGenerateMissing function.

This is a cut-and-paste CL, no behavior changes.

Notes:
- WebApplicationInfo::IconInfo becomes IconInfo. TODO for next CL:
Merge IconInfo and BitmapAndSource - they are essentially the same.

- BookmarkAppHelperExtensionServiceTest.LinkedAppIconsAreNotChanged test
was always broken (a bug in ValidateAllIconsWithURLsArePresent helper)
https://codereview.chromium.org/1066623008/patch/80001/90005

Bug: 860581
Change-Id: I0d866ac323f8dfdb409b69b22fac796155b1da23
Reviewed-on: https://chromium-review.googlesource.com/1166751
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581827}
[modify] https://crrev.com/c829901563d1d0f8c2316b16f73fc2ac256ce2ae/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/c829901563d1d0f8c2316b16f73fc2ac256ce2ae/chrome/browser/extensions/bookmark_app_helper.h
[modify] https://crrev.com/c829901563d1d0f8c2316b16f73fc2ac256ce2ae/chrome/browser/extensions/bookmark_app_helper_unittest.cc
[modify] https://crrev.com/c829901563d1d0f8c2316b16f73fc2ac256ce2ae/chrome/browser/web_applications/components/BUILD.gn
[add] https://crrev.com/c829901563d1d0f8c2316b16f73fc2ac256ce2ae/chrome/browser/web_applications/components/web_app_icon_generator.cc
[add] https://crrev.com/c829901563d1d0f8c2316b16f73fc2ac256ce2ae/chrome/browser/web_applications/components/web_app_icon_generator.h
[add] https://crrev.com/c829901563d1d0f8c2316b16f73fc2ac256ce2ae/chrome/browser/web_applications/components/web_app_icon_generator_unittest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 9

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

commit 224eb4663f904ffc0c1356fe5f1d83fc4afc2c90
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Thu Aug 09 14:26:57 2018

Revert "WebApp: Extract BookmarkAppHelper's gfx stuff into a separate file."

This reverts commit c829901563d1d0f8c2316b16f73fc2ac256ce2ae.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 581827 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2M4Mjk5MDE1NjNkMWQwZjhjMjMxNmIxNmY3M2ZjMmFjMjU2Y2UyYWUM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Linux%20MSan%20Tests/11127

Sample Failed Step: unit_tests

Original change's description:
> WebApp: Extract BookmarkAppHelper's gfx stuff into a separate file.
> 
> Extract all the low-level skia/gfx/color_util stuff as
> ResizeIconsAndGenerateMissing function.
> 
> This is a cut-and-paste CL, no behavior changes.
> 
> Notes:
> - WebApplicationInfo::IconInfo becomes IconInfo. TODO for next CL:
> Merge IconInfo and BitmapAndSource - they are essentially the same.
> 
> - BookmarkAppHelperExtensionServiceTest.LinkedAppIconsAreNotChanged test
> was always broken (a bug in ValidateAllIconsWithURLsArePresent helper)
> https://codereview.chromium.org/1066623008/patch/80001/90005
> 
> Bug: 860581
> Change-Id: I0d866ac323f8dfdb409b69b22fac796155b1da23
> Reviewed-on: https://chromium-review.googlesource.com/1166751
> Commit-Queue: Alexey Baskakov <loyso@chromium.org>
> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> Reviewed-by: Dominick Ng <dominickn@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#581827}

Change-Id: Ie69e47f17b6815e6e952e9b73284c3af4d51522c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 860581
Reviewed-on: https://chromium-review.googlesource.com/1169402

[modify] https://crrev.com/224eb4663f904ffc0c1356fe5f1d83fc4afc2c90/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/224eb4663f904ffc0c1356fe5f1d83fc4afc2c90/chrome/browser/extensions/bookmark_app_helper.h
[modify] https://crrev.com/224eb4663f904ffc0c1356fe5f1d83fc4afc2c90/chrome/browser/extensions/bookmark_app_helper_unittest.cc
[modify] https://crrev.com/224eb4663f904ffc0c1356fe5f1d83fc4afc2c90/chrome/browser/web_applications/components/BUILD.gn
[delete] https://crrev.com/e7591612da4c080af86c964d14b25c4061f5b51b/chrome/browser/web_applications/components/web_app_icon_generator.cc
[delete] https://crrev.com/e7591612da4c080af86c964d14b25c4061f5b51b/chrome/browser/web_applications/components/web_app_icon_generator.h
[delete] https://crrev.com/e7591612da4c080af86c964d14b25c4061f5b51b/chrome/browser/web_applications/components/web_app_icon_generator_unittest.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Aug 9

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

commit a86c92901aa9f21db709d7b40d1fcf0d472570d2
Author: Alexey Baskakov <loyso@chromium.org>
Date: Thu Aug 09 19:25:04 2018

Reland "WebApp: Extract BookmarkAppHelper's gfx stuff into a separate file."

This is a reland of c829901563d1d0f8c2316b16f73fc2ac256ce2ae
to revert findit revert of the above in https://chromium.googlesource.com/chromium/src/+/224eb4663f904ffc0c1356fe5f1d83fc4afc2c90
Why? Due to Gerrit outage  http://crbug.com/872722 , we are reverting this CL.
Please, re-land it after all clear is given.
If you have questions, please ask on the bug. Sorry for the inconvenience.

Original change's description:
> WebApp: Extract BookmarkAppHelper's gfx stuff into a separate file.
>
> Extract all the low-level skia/gfx/color_util stuff as
> ResizeIconsAndGenerateMissing function.
>
> This is a cut-and-paste CL, no behavior changes.
>
> Notes:
> - WebApplicationInfo::IconInfo becomes IconInfo. TODO for next CL:
> Merge IconInfo and BitmapAndSource - they are essentially the same.
>
> - BookmarkAppHelperExtensionServiceTest.LinkedAppIconsAreNotChanged test
> was always broken (a bug in ValidateAllIconsWithURLsArePresent helper)
> https://codereview.chromium.org/1066623008/patch/80001/90005
>
> Bug: 860581
> Change-Id: I0d866ac323f8dfdb409b69b22fac796155b1da23
> Reviewed-on: https://chromium-review.googlesource.com/1166751
> Commit-Queue: Alexey Baskakov <loyso@chromium.org>
> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> Reviewed-by: Dominick Ng <dominickn@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#581827}

Bug: 860581
Change-Id: If155ed6aedda2c7256f3ab707d4d335eeab26b6f
Reviewed-on: https://chromium-review.googlesource.com/1169795
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/a86c92901aa9f21db709d7b40d1fcf0d472570d2/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/a86c92901aa9f21db709d7b40d1fcf0d472570d2/chrome/browser/extensions/bookmark_app_helper.h
[modify] https://crrev.com/a86c92901aa9f21db709d7b40d1fcf0d472570d2/chrome/browser/extensions/bookmark_app_helper_unittest.cc
[modify] https://crrev.com/a86c92901aa9f21db709d7b40d1fcf0d472570d2/chrome/browser/web_applications/components/BUILD.gn
[add] https://crrev.com/a86c92901aa9f21db709d7b40d1fcf0d472570d2/chrome/browser/web_applications/components/web_app_icon_generator.cc
[add] https://crrev.com/a86c92901aa9f21db709d7b40d1fcf0d472570d2/chrome/browser/web_applications/components/web_app_icon_generator.h
[add] https://crrev.com/a86c92901aa9f21db709d7b40d1fcf0d472570d2/chrome/browser/web_applications/components/web_app_icon_generator_unittest.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Aug 9

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

commit 2a5ef9bedef9990a0b35c83a902ef545b881b4c1
Author: Dirk Pranke <dpranke@chromium.org>
Date: Thu Aug 09 22:11:39 2018

Revert "Reland "WebApp: Extract BookmarkAppHelper's gfx stuff into a separate file.""

This reverts commit a86c92901aa9f21db709d7b40d1fcf0d472570d2.

Reason for revert: We actually want this change reverted :). The original CL was bad, the revert was a manual thing done by the sheriff. The GoB fix then re-landed this, and now we want to reland it again.
Original change's description:
> Reland "WebApp: Extract BookmarkAppHelper's gfx stuff into a separate file."
> 
> This is a reland of c829901563d1d0f8c2316b16f73fc2ac256ce2ae
> to revert findit revert of the above in https://chromium.googlesource.com/chromium/src/+/224eb4663f904ffc0c1356fe5f1d83fc4afc2c90
> Why? Due to Gerrit outage  http://crbug.com/872722 , we are reverting this CL.
> Please, re-land it after all clear is given.
> If you have questions, please ask on the bug. Sorry for the inconvenience.
> 
> Original change's description:
> > WebApp: Extract BookmarkAppHelper's gfx stuff into a separate file.
> >
> > Extract all the low-level skia/gfx/color_util stuff as
> > ResizeIconsAndGenerateMissing function.
> >
> > This is a cut-and-paste CL, no behavior changes.
> >
> > Notes:
> > - WebApplicationInfo::IconInfo becomes IconInfo. TODO for next CL:
> > Merge IconInfo and BitmapAndSource - they are essentially the same.
> >
> > - BookmarkAppHelperExtensionServiceTest.LinkedAppIconsAreNotChanged test
> > was always broken (a bug in ValidateAllIconsWithURLsArePresent helper)
> > https://codereview.chromium.org/1066623008/patch/80001/90005
> >
> > Bug: 860581
> > Change-Id: I0d866ac323f8dfdb409b69b22fac796155b1da23
> > Reviewed-on: https://chromium-review.googlesource.com/1166751
> > Commit-Queue: Alexey Baskakov <loyso@chromium.org>
> > Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> > Reviewed-by: Dominick Ng <dominickn@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#581827}
> 
> Bug: 860581
> Change-Id: If155ed6aedda2c7256f3ab707d4d335eeab26b6f
> Reviewed-on: https://chromium-review.googlesource.com/1169795
> Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>

TBR=ortuno@chromium.org,loyso@chromium.org,tandrii@chromium.org,dominickn@chromium.org

Change-Id: I8c9475f3ed6ddb87bc09c0b1f8317147134ecbb1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 860581
Reviewed-on: https://chromium-review.googlesource.com/1169928
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581908}
[modify] https://crrev.com/2a5ef9bedef9990a0b35c83a902ef545b881b4c1/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/2a5ef9bedef9990a0b35c83a902ef545b881b4c1/chrome/browser/extensions/bookmark_app_helper.h
[modify] https://crrev.com/2a5ef9bedef9990a0b35c83a902ef545b881b4c1/chrome/browser/extensions/bookmark_app_helper_unittest.cc
[modify] https://crrev.com/2a5ef9bedef9990a0b35c83a902ef545b881b4c1/chrome/browser/web_applications/components/BUILD.gn
[delete] https://crrev.com/c53e53438d29ce4c55c6df6fd1d646ad2f5047f6/chrome/browser/web_applications/components/web_app_icon_generator.cc
[delete] https://crrev.com/c53e53438d29ce4c55c6df6fd1d646ad2f5047f6/chrome/browser/web_applications/components/web_app_icon_generator.h
[delete] https://crrev.com/c53e53438d29ce4c55c6df6fd1d646ad2f5047f6/chrome/browser/web_applications/components/web_app_icon_generator_unittest.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Aug 13

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

commit 623b7070a08469c236a3c45a76b61d8bb2ba8a28
Author: Alexey Baskakov <loyso@chromium.org>
Date: Mon Aug 13 01:45:20 2018

WebApp: Extract BookmarkAppHelper's gfx stuff into a separate file.

Extract all the low-level skia/gfx/color_util stuff as
ResizeIconsAndGenerateMissing function.

This is a cut-and-paste CL, no behavior changes.

Notes:
- WebApplicationInfo::IconInfo becomes IconInfo. TODO for next CL:
Merge IconInfo and BitmapAndSource - they are essentially the same.

- BookmarkAppHelperExtensionServiceTest.LinkedAppIconsAreNotChanged test
was always broken (a bug in ValidateAllIconsWithURLsArePresent helper)
https://codereview.chromium.org/1066623008/patch/80001/90005

This is a re-land of:
https://chromium-review.googlesource.com/c/chromium/src/+/1166751
MSan issue solved, ouput parameters initialized everywhere:
SkColor generated_icon_color = SK_ColorTRANSPARENT;

TBR=ortuno@chromium.org

Bug: 860581
Change-Id: Ib26c765083e67e3a1c938a49994a5f5d5ecc8265
Reviewed-on: https://chromium-review.googlesource.com/1170665
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582491}
[modify] https://crrev.com/623b7070a08469c236a3c45a76b61d8bb2ba8a28/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/623b7070a08469c236a3c45a76b61d8bb2ba8a28/chrome/browser/extensions/bookmark_app_helper.h
[modify] https://crrev.com/623b7070a08469c236a3c45a76b61d8bb2ba8a28/chrome/browser/extensions/bookmark_app_helper_unittest.cc
[modify] https://crrev.com/623b7070a08469c236a3c45a76b61d8bb2ba8a28/chrome/browser/web_applications/components/BUILD.gn
[add] https://crrev.com/623b7070a08469c236a3c45a76b61d8bb2ba8a28/chrome/browser/web_applications/components/web_app_icon_generator.cc
[add] https://crrev.com/623b7070a08469c236a3c45a76b61d8bb2ba8a28/chrome/browser/web_applications/components/web_app_icon_generator.h
[add] https://crrev.com/623b7070a08469c236a3c45a76b61d8bb2ba8a28/chrome/browser/web_applications/components/web_app_icon_generator_unittest.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Aug 13

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

commit fcdcd05ace7bd29d81f4fa4865304dc5d22acb3b
Author: Alexey Baskakov <loyso@chromium.org>
Date: Mon Aug 13 07:22:24 2018

WebApp: Clean up WebAppIconGeneratorTest unit tests.

No behavior change.

Bug: 860581
Change-Id: I808616a351c2f7912ebab1e6f052e00c10e4107a
Reviewed-on: https://chromium-review.googlesource.com/1172171
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582520}
[modify] https://crrev.com/fcdcd05ace7bd29d81f4fa4865304dc5d22acb3b/chrome/browser/web_applications/components/web_app_icon_generator_unittest.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Aug 14

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

commit 448040b5344a4185a4411a106c69fe4594023644
Author: Alexey Baskakov <loyso@chromium.org>
Date: Tue Aug 14 01:23:10 2018

WebApp: Inline BookmarkAppHelper::ResizeIconsAndGenerateMissing helper.

It is a thin wrapper for web_app::ResizeIconsAndGenerateMissing anyway.

Bug: 860581
Change-Id: I3e6ff80dddf34ca08affe2fa1929f70702c7cee9
Reviewed-on: https://chromium-review.googlesource.com/1172238
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582782}
[modify] https://crrev.com/448040b5344a4185a4411a106c69fe4594023644/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/448040b5344a4185a4411a106c69fe4594023644/chrome/browser/extensions/bookmark_app_helper.h

Status: Assigned (was: Started)
Blocking: 877898

Sign in to add a comment