Web app: set a flag on the Intent if the icon was generated by Chromium.

Web app: set a flag on the Intent if the icon was generated by Chromium.

This is in order to prevent Chromium to then re-use that icon for
anything related to web app branding.

BUG=546212
Committed: https://crrev.com/bc6e8797d0454d21c3f5ee8e52011fe63228ad7a
Cr-Commit-Position: refs/heads/master@{#355634}

[mlamouri (slow - plz ping), 2015-10-21 22:08:35]

Description was changed from

==========
Web app: set a flag on the Intent if the icon was generated by Chromium.

This is in order to prevent Chromium to then re-use that icon for
anything related to web app branding.

WIP

BUG=TODO
==========

to

==========
Web app: set a flag on the Intent if the icon was generated by Chromium.

This is in order to prevent Chromium to then re-use that icon for
anything related to web app branding.

BUG=TODO
==========

[mlamouri (slow - plz ping), 2015-10-21 22:12:24]

Description was changed from

==========
Web app: set a flag on the Intent if the icon was generated by Chromium.

This is in order to prevent Chromium to then re-use that icon for
anything related to web app branding.

BUG=TODO
==========

to

==========
Web app: set a flag on the Intent if the icon was generated by Chromium.

This is in order to prevent Chromium to then re-use that icon for
anything related to web app branding.

BUG=546212
==========

[mlamouri (slow - plz ping), 2015-10-21 22:13:00]

mlamouri@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[mlamouri (slow - plz ping), 2015-10-21 22:13:01]

Dan. PTAL. This is what I'm going to use to base the new splash screen
adaptative layout.

[gone, 2015-10-21 23:35:57]

mostly lgtm, just naming nits that affect all the files

https://codereview.chromium.org/1420743002/diff/20001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
(right):

https://codereview.chromium.org/1420743002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java:67:
public static final String EXTRA_GENERATED_ICON =
"org.chromium.chrome.browser.generated_icon";
nit: GENERATED_ICON sounds like an actual icon.  IS_ICON_GENERATED?

https://codereview.chromium.org/1420743002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java:74:
private static final String TAG = "cr_Shortcuts";
nit: You don't even need the cr_ anymore, AFAIK.  It's automatic.

https://codereview.chromium.org/1420743002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java:250: *
will be used, else we draw our own.
This comment is out of date, I think, now that there's a separate function for
generating one.

https://codereview.chromium.org/1420743002/diff/20001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappInfo.java
(right):

https://codereview.chromium.org/1420743002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappInfo.java:74:
boolean generatedIcon = IntentUtils.safeGetBooleanExtra(intent,
same naming comment as above

https://codereview.chromium.org/1420743002/diff/20001/chrome/browser/android/...
File chrome/browser/android/shortcut_helper.cc (right):

https://codereview.chromium.org/1420743002/diff/20001/chrome/browser/android/...
chrome/browser/android/shortcut_helper.cc:165: bool* generated) {
is_generated

https://codereview.chromium.org/1420743002/diff/20001/chrome/browser/android/...
File chrome/browser/android/shortcut_helper.h (right):

https://codereview.chromium.org/1420743002/diff/20001/chrome/browser/android/...
chrome/browser/android/shortcut_helper.h:56: // Modify the given icon to matches
the launcher requirements and returns the
to match

requirements, then returns

[mlamouri (slow - plz ping), 2015-10-22 14:37:07]

Review comments applied. This is waiting for the dependent CLs to land now.

https://codereview.chromium.org/1420743002/diff/20001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
(right):

https://codereview.chromium.org/1420743002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java:67:
public static final String EXTRA_GENERATED_ICON =
"org.chromium.chrome.browser.generated_icon";
On 2015/10/21 at 23:35:57, dfalcantara wrote:
> nit: GENERATED_ICON sounds like an actual icon.  IS_ICON_GENERATED?

You are correct. I actually thought about that and forgot...
https://imgflip.com/i/sy8km

https://codereview.chromium.org/1420743002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java:74:
private static final String TAG = "cr_Shortcuts";
On 2015/10/21 at 23:35:57, dfalcantara wrote:
> nit: You don't even need the cr_ anymore, AFAIK.  It's automatic.

Done. Renamed to ShortcutHelper.

https://codereview.chromium.org/1420743002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java:250: *
will be used, else we draw our own.
On 2015/10/21 at 23:35:57, dfalcantara wrote:
> This comment is out of date, I think, now that there's a separate function for
generating one.

Done.

https://codereview.chromium.org/1420743002/diff/20001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappInfo.java
(right):

https://codereview.chromium.org/1420743002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappInfo.java:74:
boolean generatedIcon = IntentUtils.safeGetBooleanExtra(intent,
On 2015/10/21 at 23:35:57, dfalcantara wrote:
> same naming comment as above

Done.

https://codereview.chromium.org/1420743002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappInfo.java:234:
public boolean hasGeneratedIcon() {
I renamed this to isIconGenerated() to match the name.

https://codereview.chromium.org/1420743002/diff/20001/chrome/browser/android/...
File chrome/browser/android/shortcut_helper.cc (right):

https://codereview.chromium.org/1420743002/diff/20001/chrome/browser/android/...
chrome/browser/android/shortcut_helper.cc:165: bool* generated) {
On 2015/10/21 at 23:35:57, dfalcantara wrote:
> is_generated

Done.

https://codereview.chromium.org/1420743002/diff/20001/chrome/browser/android/...
File chrome/browser/android/shortcut_helper.h (right):

https://codereview.chromium.org/1420743002/diff/20001/chrome/browser/android/...
chrome/browser/android/shortcut_helper.h:56: // Modify the given icon to matches
the launcher requirements and returns the
On 2015/10/21 at 23:35:57, dfalcantara wrote:
> to match
> 
> requirements, then returns

Done.

https://codereview.chromium.org/1420743002/diff/20001/chrome/browser/android/...
File chrome/browser/android/shortcut_info.h (right):

https://codereview.chromium.org/1420743002/diff/20001/chrome/browser/android/...
chrome/browser/android/shortcut_info.h:48: bool generated_icon;
I've renamed this to is_generated_icon.

[mlamouri (slow - plz ping), 2015-10-22 19:56:56]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2015-10-22 19:56:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/1420743002/#ps40001
(title: "review comments")

[commit-bot: I haz the power, 2015-10-22 19:57:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420743002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420743002/40001

[commit-bot: I haz the power, 2015-10-22 20:41:20]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-22 20:42:12]

Patchset 3 (id:??) landed as
https://crrev.com/bc6e8797d0454d21c3f5ee8e52011fe63228ad7a
Cr-Commit-Position: refs/heads/master@{#355634}