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

Issue 2872473002: Merge ChromeShortcutManager into ShortcutHelper. (Closed)

Created:
3 years, 7 months ago by Marti Wong
Modified:
3 years, 7 months ago
Reviewers:
dominickn, gone
CC:
chromium-reviews, dominickn+watch_chromium.org, pkotwicz+watch_chromium.org, zpeng+watch_chromium.org, agrieve+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Merge ChromeShortcutManager into ShortcutHelper. ChromeShortcutManager was created for pre-O/O separate compilation. After upstreaming of ChromeShortcutManagerInternal (crrev.com/2845803003 and https://chrome-internal-review.googlesource.com/c/367589) , the content in this class could be merged into ShortcutHelper to make the code cleaner and less confusing. BUG=708360 Review-Url: https://codereview.chromium.org/2872473002 Cr-Commit-Position: refs/heads/master@{#470773} Committed: https://chromium.googlesource.com/chromium/src/+/5b2fff30d156d502cde5f0e9ea52cdda9071375f

Patch Set 1 #

Total comments: 2

Patch Set 2 : fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+138 lines, -194 lines) Patch
M chrome/android/java/src/org/chromium/chrome/browser/AppHooks.java View 2 chunks +0 lines, -7 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java View 1 14 chunks +138 lines, -8 lines 0 comments Download
D chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeShortcutManager.java View 1 chunk +0 lines, -178 lines 0 comments Download
M chrome/android/java_sources.gni View 1 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 32 (18 generated)
Marti Wong
PTAL thanks~!
3 years, 7 months ago (2017-05-08 04:44:17 UTC) #3
dominickn
lgtm, thanks so much for this Marti! It's good to have all of this code ...
3 years, 7 months ago (2017-05-08 04:51:22 UTC) #4
gone
lgtm
3 years, 7 months ago (2017-05-08 17:20:25 UTC) #9
Marti Wong
Thanks for the help~! commiting. https://codereview.chromium.org/2872473002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java File chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java (right): https://codereview.chromium.org/2872473002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java#newcode100 chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java:100: // have to update ...
3 years, 7 months ago (2017-05-09 04:36:42 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2872473002/20001
3 years, 7 months ago (2017-05-09 04:38:02 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/421566)
3 years, 7 months ago (2017-05-09 06:34:43 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2872473002/20001
3 years, 7 months ago (2017-05-09 23:30:28 UTC) #17
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; ...
3 years, 7 months ago (2017-05-10 01:34:54 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2872473002/20001
3 years, 7 months ago (2017-05-10 01:55:00 UTC) #21
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; ...
3 years, 7 months ago (2017-05-10 03:57:38 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2872473002/20001
3 years, 7 months ago (2017-05-10 07:34:45 UTC) #25
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/440458)
3 years, 7 months ago (2017-05-10 09:04:46 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2872473002/20001
3 years, 7 months ago (2017-05-11 00:59:22 UTC) #29
commit-bot: I haz the power
3 years, 7 months ago (2017-05-11 02:09:56 UTC) #32
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/5b2fff30d156d502cde5f0e9ea52...

Powered by Google App Engine
This is Rietveld 408576698