Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(342)

Issue 12178030: [mac] Delete app shortcuts on app uninstall. (Closed)

Created:
7 years, 10 months ago by jeremya
Modified:
7 years, 10 months ago
CC:
chromium-reviews, Aaron Boodman, sail+watch_chromium.org, chromium-apps-reviews_chromium.org, chrome-apps-syd-reviews_chromium.org
Visibility:
Public.

Description

[mac] Delete app shortcuts on app uninstall. When an app is uninstalled, find the .app bundle it's associated with using Launch Services, and delete it. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=182077

Patch Set 1 #

Total comments: 16

Patch Set 2 : comments #

Total comments: 7

Patch Set 3 : comments #

Total comments: 6

Patch Set 4 : nits #

Patch Set 5 : rebase #

Patch Set 6 : fix rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -17 lines) Patch
M chrome/browser/extensions/app_shortcut_manager.cc View 1 2 3 4 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/web_applications/web_app_mac.mm View 1 2 3 4 5 3 chunks +26 lines, -3 lines 0 comments Download
M chrome/service/chrome_service_application_mac.mm View 1 2 3 3 chunks +11 lines, -11 lines 0 comments Download

Messages

Total messages: 17 (0 generated)
jeremya
benwells- apps-related review and OWNERS avi- for the mac stuff, specifically the LS usage
7 years, 10 months ago (2013-02-05 00:03:21 UTC) #1
Avi (use Gerrit)
https://codereview.chromium.org/12178030/diff/1/chrome/browser/extensions/app_shortcut_manager.cc File chrome/browser/extensions/app_shortcut_manager.cc (right): https://codereview.chromium.org/12178030/diff/1/chrome/browser/extensions/app_shortcut_manager.cc#newcode86 chrome/browser/extensions/app_shortcut_manager.cc:86: #endif Can you label some of these? It's hard ...
7 years, 10 months ago (2013-02-05 00:19:25 UTC) #2
jeremya
https://codereview.chromium.org/12178030/diff/1/chrome/browser/extensions/app_shortcut_manager.cc File chrome/browser/extensions/app_shortcut_manager.cc (right): https://codereview.chromium.org/12178030/diff/1/chrome/browser/extensions/app_shortcut_manager.cc#newcode86 chrome/browser/extensions/app_shortcut_manager.cc:86: #endif On 2013/02/05 00:19:25, Avi wrote: > Can you ...
7 years, 10 months ago (2013-02-05 02:38:14 UTC) #3
Avi (use Gerrit)
https://codereview.chromium.org/12178030/diff/1/chrome/browser/web_applications/web_app_mac.mm File chrome/browser/web_applications/web_app_mac.mm (right): https://codereview.chromium.org/12178030/diff/1/chrome/browser/web_applications/web_app_mac.mm#newcode269 chrome/browser/web_applications/web_app_mac.mm:269: NULL, &ref, kDontWantURL); Would you be so kind as ...
7 years, 10 months ago (2013-02-05 04:01:40 UTC) #4
benwells
c/b/e lgtm
7 years, 10 months ago (2013-02-05 04:08:11 UTC) #5
jeremya
+ajwong for chrome/service OWNERS. Looks like the original author of that code has left google. ...
7 years, 10 months ago (2013-02-05 04:28:52 UTC) #6
Avi (use Gerrit)
lgtm Who wrote that cloud print code? It's leaky as hell. https://codereview.chromium.org/12178030/diff/1/chrome/browser/web_applications/web_app_mac.mm File chrome/browser/web_applications/web_app_mac.mm (right): ...
7 years, 10 months ago (2013-02-05 05:15:47 UTC) #7
jeremya
according to git blame, abeera@google.com wrote it, but they don't seem to be working at ...
7 years, 10 months ago (2013-02-05 05:42:15 UTC) #8
Avi (use Gerrit)
My "who wrote this" question was more rhetorical, but oh well. SLGTM
7 years, 10 months ago (2013-02-05 15:59:10 UTC) #9
awong
service rubberstamp lgtm AFAICT, this is just fixing a pretty obviously looking memory leak and ...
7 years, 10 months ago (2013-02-06 21:10:00 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jeremya@chromium.org/12178030/17001
7 years, 10 months ago (2013-02-12 08:05:59 UTC) #11
commit-bot: I haz the power
Failed to apply patch for chrome/browser/web_applications/web_app_mac.mm: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
7 years, 10 months ago (2013-02-12 08:06:00 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jeremya@chromium.org/12178030/8003
7 years, 10 months ago (2013-02-12 08:24:02 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jeremya@chromium.org/12178030/19003
7 years, 10 months ago (2013-02-12 08:44:37 UTC) #14
commit-bot: I haz the power
Retried try job too often on mac_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&number=98611
7 years, 10 months ago (2013-02-12 09:47:03 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jeremya@chromium.org/12178030/19003
7 years, 10 months ago (2013-02-12 21:26:46 UTC) #16
commit-bot: I haz the power
7 years, 10 months ago (2013-02-13 00:29:43 UTC) #17
Retried try job too often on mac_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

Powered by Google App Engine
This is Rietveld 408576698