[Custom Tabs]Record warnings if given action button bitmap's size out of bound

[Custom Tabs]Record warnings if given action button bitmap's size out of bound

Having a logcat warning is good for developers to debug, as some of them
may not know we restrict the size of the bitmap.

BUG=540023
Committed: https://crrev.com/8992c9af1724c10caaa1581423d133c6f2abc4a5
Cr-Commit-Position: refs/heads/master@{#352771}

[Ian Wen, 2015-10-06 22:07:56]

ianwen@chromium.org changed reviewers:
+ tedchoc@chromium.org, yusufo@chromium.org

[Ian Wen, 2015-10-06 22:07:57]

PTAL.

[Ted C, 2015-10-06 23:23:49]

https://codereview.chromium.org/1385283002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/ActionButtonParams.java
(right):

https://codereview.chromium.org/1385283002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/ActionButtonParams.java:118:
+ " height: " + bitmap.getHeight());
I would log this in the check bounds method below only so you can print the
maximum allowed size as part of this message (toolbar_icon_height).

Or is there a doc that describes the maximum allowed size somewhere that we can
link to?  If we can do that, that is better than the method below (since the
size depends on density, the message could be misleading if we didn't print the
DP value, also the scaled value is a bit hard to describe).

[Yusuf, 2015-10-06 23:44:02]

https://codereview.chromium.org/1385283002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/ActionButtonParams.java
(right):

https://codereview.chromium.org/1385283002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/ActionButtonParams.java:118:
+ " height: " + bitmap.getHeight());
On 2015/10/06 23:23:49, Ted C wrote:
> I would log this in the check bounds method below only so you can print the
> maximum allowed size as part of this message (toolbar_icon_height).
> 
> Or is there a doc that describes the maximum allowed size somewhere that we
can
> link to?  If we can do that, that is better than the method below (since the
> size depends on density, the message could be misleading if we didn't print
the
> DP value, also the scaled value is a bit hard to describe).

Here is the spec:
https://developer.android.com/reference/android/support/customtabs/CustomTabs...

[Ian Wen, 2015-10-06 23:47:22]

https://codereview.chromium.org/1385283002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/ActionButtonParams.java
(right):

https://codereview.chromium.org/1385283002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/ActionButtonParams.java:118:
+ " height: " + bitmap.getHeight());
On 2015/10/06 23:44:02, Yusuf wrote:
> On 2015/10/06 23:23:49, Ted C wrote:
> > I would log this in the check bounds method below only so you can print the
> > maximum allowed size as part of this message (toolbar_icon_height).
> > 
> > Or is there a doc that describes the maximum allowed size somewhere that we
> can
> > link to?  If we can do that, that is better than the method below (since the
> > size depends on density, the message could be misleading if we didn't print
> the
> > DP value, also the scaled value is a bit hard to describe).
> 
> Here is the spec:
>
https://developer.android.com/reference/android/support/customtabs/CustomTabs...

Great. Could I say "Action Button's Icon size not acceptable. Please refer to
http://......"?

[Ted C, 2015-10-06 23:48:53]

On 2015/10/06 23:47:22, Ian Wen wrote:
>
https://codereview.chromium.org/1385283002/diff/1/chrome/android/java/src/org...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/customtabs/ActionButtonParams.java
> (right):
> 
>
https://codereview.chromium.org/1385283002/diff/1/chrome/android/java/src/org...
>
chrome/android/java/src/org/chromium/chrome/browser/customtabs/ActionButtonParams.java:118:
> + " height: " + bitmap.getHeight());
> On 2015/10/06 23:44:02, Yusuf wrote:
> > On 2015/10/06 23:23:49, Ted C wrote:
> > > I would log this in the check bounds method below only so you can print
the
> > > maximum allowed size as part of this message (toolbar_icon_height).
> > > 
> > > Or is there a doc that describes the maximum allowed size somewhere that
we
> > can
> > > link to?  If we can do that, that is better than the method below (since
the
> > > size depends on density, the message could be misleading if we didn't
print
> > the
> > > DP value, also the scaled value is a bit hard to describe).
> > 
> > Here is the spec:
> >
>
https://developer.android.com/reference/android/support/customtabs/CustomTabs...
> 
> Great. Could I say "Action Button's Icon size not acceptable. Please refer to
> http://......"?

sgtm

[Ian Wen, 2015-10-07 00:09:25]

https://codereview.chromium.org/1385283002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/customtabs/ActionButtonParams.java
(right):

https://codereview.chromium.org/1385283002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/customtabs/ActionButtonParams.java:118:
+ " height: " + bitmap.getHeight());
On 2015/10/06 23:47:22, Ian Wen wrote:
> On 2015/10/06 23:44:02, Yusuf wrote:
> > On 2015/10/06 23:23:49, Ted C wrote:
> > > I would log this in the check bounds method below only so you can print
the
> > > maximum allowed size as part of this message (toolbar_icon_height).
> > > 
> > > Or is there a doc that describes the maximum allowed size somewhere that
we
> > can
> > > link to?  If we can do that, that is better than the method below (since
the
> > > size depends on density, the message could be misleading if we didn't
print
> > the
> > > DP value, also the scaled value is a bit hard to describe).
> > 
> > Here is the spec:
> >
>
https://developer.android.com/reference/android/support/customtabs/CustomTabs...
> 
> Great. Could I say "Action Button's Icon size not acceptable. Please refer to
> http://......"?

Done.

[Ted C, 2015-10-07 03:15:08]

lgtm

[Ian Wen, 2015-10-07 03:37:19]

The CQ bit was checked by ianwen@chromium.org

[commit-bot: I haz the power, 2015-10-07 03:39:38]

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

[commit-bot: I haz the power, 2015-10-07 04:29:44]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-07 04:30:45]

Patchset 2 (id:??) landed as
https://crrev.com/8992c9af1724c10caaa1581423d133c6f2abc4a5
Cr-Commit-Position: refs/heads/master@{#352771}