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

Issue 708360 link

Starred by 1 user
Status: Fixed
Owner:
█ martiw@chromium.org
Last visit > 30 days ago
Closed: May 2017
Cc:
dominickn@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Task



Sign in to add a comment

Upstream ChromeShortcutManagerInternal

Project Member Reported by dominickn@chromium.org, Apr 5 2017 
Android O is now public so we should upstream this code to the public repository. In this process, we should consider if we can now just merge the functionality into ShortcutHelper.

Though merging might not be possible due to SDK dependencies (we can only call requestPinShortcut() if we're building against the O SDK, which we don't currently do.
Project Member

Comment 1 by bugdroid1@chromium.org, May 2 2017 

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

commit 37235f0ea4c66711ae8cc694978bfb98657dd0c6
Author: martiw <martiw@chromium.org>
Date: Tue May 02 01:14:32 2017

Upstream ChromeShortcutManagerInternal using reflection.
This CL upstreams the internal Android O code using reflection.
I learned from the 2 upstreaming examples below:
crrev.com/2834633002/
crrev.com/2765003002/

Added some TODOs for removing those reflection when compileSdk >=O

BUG= 708360 

Review-Url: https://codereview.chromium.org/2845803003
Cr-Commit-Position: refs/heads/master@{#468523}

[modify] https://crrev.com/37235f0ea4c66711ae8cc694978bfb98657dd0c6/chrome/android/java/src/org/chromium/chrome/browser/AppHooks.java
[modify] https://crrev.com/37235f0ea4c66711ae8cc694978bfb98657dd0c6/chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeShortcutManager.java
Project Member

Comment 2 by bugdroid1@chromium.org, May 3 2017 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/e227aa0283968dec67ef15fae8a5d32778e401bd

commit e227aa0283968dec67ef15fae8a5d32778e401bd
Author: Marti Wong <martiw@google.com>
Date: Wed May 03 23:53:29 2017

Comment 3 by martiw@chromium.org, May 4 2017

Labels: -Pri-2 Pri-3
The final step of this bug is do merge ChromeShortcutManager into ShortcutHelper.
To make the code cleaner and less confusing.
(ChromeShortcutManager was created for pre-O/O separate compilation, which is no longer needed.)
This will be done in another CL.

Comment 4 by dominickn@chromium.org, May 9 2017

Cc: -martiw@chromium.org
Owner: martiw@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, May 11 2017 

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

commit 5b2fff30d156d502cde5f0e9ea52cdda9071375f
Author: martiw <martiw@chromium.org>
Date: Thu May 11 02:09:34 2017

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}

[modify] https://crrev.com/5b2fff30d156d502cde5f0e9ea52cdda9071375f/chrome/android/java/src/org/chromium/chrome/browser/AppHooks.java
[modify] https://crrev.com/5b2fff30d156d502cde5f0e9ea52cdda9071375f/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
[delete] https://crrev.com/1aa98d7cf548e42ef1a75828caa6d6831445172b/chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeShortcutManager.java
[modify] https://crrev.com/5b2fff30d156d502cde5f0e9ea52cdda9071375f/chrome/android/java_sources.gni

Comment 6 by martiw@chromium.org, May 16 2017

Status: Fixed (was: Available)
►

Sign in to add a comment