Update the AppBannerView appearance

Update the AppBannerView appearance

* Updates the AppBannerView assets to the ones provided by
  the Android guys and deletes the old ones.
* Updates AppBannerView calculations to account for the new
  assets and their padding values.
* Adds a RatingView class that displays the given number
  of stars in increments of 0.5.
* Adds an event to the SwipableOverlayView to respond to
  clicking on the View (and none of its buttons).
* AppBannerView's install button changes based on whether an
  app is being installed, installed, or being advertised.
* Changes the layout size on Nexus 7s so that it uses a
  phone-sized card instead of a tablet one in landscape mode.
* Introduces an AppData structure for simply storing all of
  an app's data in one place.

! Adds strings for the "installing" and "open" states.

NOTRY=true
BUG=341556
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=252327

[gone, 2014-02-14 19:17:14]

Moved some stuff around to clean up a different CL.

[gone, 2014-02-19 03:24:22]

Haven't seen any review activity so I just piled on more changes requested by
the Android UX guys into this one.

[newt (away), 2014-02-19 21:52:49]

several comments, all minor though

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
File chrome/android/java/res/layout/app_banner_view.xml (right):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
chrome/android/java/res/layout/app_banner_view.xml:10:
android:background="@drawable/card_background_default"
so you're just adding xxhdpi assets and letting Android scale them down for
other resolutions?

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
chrome/android/java/res/layout/app_banner_view.xml:45:
android:src="@drawable/logo_play_bw"
"google_play_logo" seems like a clearer name

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
File chrome/android/java/res/values-h720dp/dimens.xml (right):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
chrome/android/java/res/values-h720dp/dimens.xml:12: <dimen
name="app_banner_button_height">34dp</dimen>
What if the system font size is HUGE? Is the button still big enough? You might
be safer specifying the button height in sp, or just specifying the padding and
letting the height be wrap_content.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
File chrome/android/java/res/values-sw600dp-v17/styles.xml (left):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
chrome/android/java/res/values-sw600dp-v17/styles.xml:1: <?xml version="1.0"
encoding="utf-8"?>
are you deleting this file? rietveld seems to think you're deleting the
contents, but keeping the file...

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
File chrome/android/java/res/values-v17/styles.xml (right):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
chrome/android/java/res/values-v17/styles.xml:7: <resources>
I'd dedupe this file and values-h720dp-v17/styles.xml. Change the font sizes in
this file to @dimen/whatever_font_size, define different font sizes depending on
the screen size, and delete values-h720dp-v17/styles.xml.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
chrome/android/java/res/values-v17/styles.xml:20: <style
name="AppBannerOpenButton">
you can use style inheritance to here to ensure that AppBannerOpenButton and
AppBannerInstallButton are the same, except for color. E.g.

    <style name="AppBannerOpenButton" parent="@style/AppBannerInstallButton">
        <item name="android:textColor">#737373</item>
    </style>

Or define a comment parent and let both styles inherit from that.

OR... have a single style called "AppBannerButton" and set the color of each
button independently.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java
(right):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:63:
private Observer mObserver;
For clarity's sake, can you change this to:

    private AppBannerView.Observer mObserver;

?

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:97:
public static AppBannerView create(ContentView contentView, Observer observer,
AppData data) {
yay, shorter param list!

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:117:
// Store the Drawable's padding.
Huh? Which Drawable's padding?

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:137:
// Cache the banner dimensions, adjusting margins for drop shadows defined in
the Drawable.
still confused :/ ... Which Drawable?

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:159:
setPadding(paddingStart, paddingTop, paddingEnd, paddingBottom);
Do you mean:

    ApiCompatibilityUtils.setPaddingRelative()?

Or are you assuming the left and right padding are the same (in which case I'd
just use a single variable called paddingSide)?

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:218:
AppData getAppData() {
return a const reference:

    const AppData& getAppData() const

(once you add support for that to the JVM)

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:239:
mButtonView.setText(mAppData.installButtonText);
Can mAppData.installButtonText have different values? I'd expect it always to
say "Install", but it seems like that's not the case.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:249:

remove extra line

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:282:

I'd explain why you need to override onConfigurationChanged(), and what
configuration change you're looking out for.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:295:
AppBannerView cannibalized =
A+ for variable naming

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:297:
while (cannibalized.getChildCount() > 0) {
Which child views depend on the screen size? Do you need to replace all the
child views, or could you just replace one or two of them?

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:407:
Rect bounds = new Rect();
I bet android lint will complain about "new Rect()" inside onLayout()

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/banners/AppData.java
(right):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppData.java:34: //
Rating.
// Rating, between 0 and 5.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/banners/RatingView.java
(right):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/RatingView.java:21:
* Displays a set of stars representing a rating for an app.
Yay! Class-level JavaDoc!

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/RatingView.java:27:
private static Bitmap sStarFull;
advantage of statics: If many RatingViews exist simultaneously, they can use the
same Bitmaps and hence save memory and decoding time.

disadvantage of statics: We'll keep this memory around forever, even (in the
common case) where no more app banners will ever be shown. Plus, findbugs may
chastise you for thread safety issues here, though that's not an issue IMO.

I'd lean towards making these member variables instead of statics.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/RatingView.java:56:
Resources res = getContext().getResources();
Simpler:

    Resources res = getResources();

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/RatingView.java:58:
Bitmap starHalf = BitmapFactory.decodeResource(res,
R.drawable.btn_star_mini_half);
suggestion for simplifying this a bit:

    sStarHalf = BitmapFactory.decodeResource(res,
R.drawable.btn_star_mini_half);
    if (!mIsLayoutLTR) {
        ...
        sStartHalf = Bitmap.createBitmap(sStarHalf, 0, 0, width, height, m,
false);
    }

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/RatingView.java:77:
int starDimension = canvas.getHeight();
Is this the 5th dimension?? Just an opinion, but, "starSize" makes more sense to
me.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/banners/SwipableOverlayView.java
(right):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/SwipableOverlayView.java:458:
protected void onViewClicked() {
what now?

https://codereview.chromium.org/141853007/diff/500001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/141853007/diff/500001/chrome/app/generated_re...
chrome/app/generated_resources.grd:15111: +    <if expr="is_android">
If these strings are only used in Java, move them to android_chrome_strings.grd.
If they live in generated_resources.grd, they get generated as *both* C++ and
Java strings.

[gone, 2014-02-19 23:31:43]

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
File chrome/android/java/res/layout/app_banner_view.xml (right):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
chrome/android/java/res/layout/app_banner_view.xml:10:
android:background="@drawable/card_background_default"
On 2014/02/19 21:52:49, newt wrote:
> so you're just adding xxhdpi assets and letting Android scale them down for
> other resolutions?

Guess so, yeah.  That's the only asset size the UX guy sent over.  Saves space
on our end, anyway.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
chrome/android/java/res/layout/app_banner_view.xml:45:
android:src="@drawable/logo_play_bw"
On 2014/02/19 21:52:49, newt wrote:
> "google_play_logo" seems like a clearer name

Agreed -- renaming his asset.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
File chrome/android/java/res/values-h720dp/dimens.xml (right):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
chrome/android/java/res/values-h720dp/dimens.xml:12: <dimen
name="app_banner_button_height">34dp</dimen>
On 2014/02/19 21:52:49, newt wrote:
> What if the system font size is HUGE? Is the button still big enough? You
might
> be safer specifying the button height in sp, or just specifying the padding
and
> letting the height be wrap_content.

Made it define a min height instead.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
File chrome/android/java/res/values-sw600dp-v17/styles.xml (left):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
chrome/android/java/res/values-sw600dp-v17/styles.xml:1: <?xml version="1.0"
encoding="utf-8"?>
On 2014/02/19 21:52:49, newt wrote:
> are you deleting this file? rietveld seems to think you're deleting the
> contents, but keeping the file...

Supposed to be deleted :/  Not sure why it's not.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
File chrome/android/java/res/values-v17/styles.xml (right):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
chrome/android/java/res/values-v17/styles.xml:7: <resources>
On 2014/02/19 21:52:49, newt wrote:
> I'd dedupe this file and values-h720dp-v17/styles.xml. Change the font sizes
in
> this file to @dimen/whatever_font_size, define different font sizes depending
on
> the screen size, and delete values-h720dp-v17/styles.xml.

Nuked.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/res...
chrome/android/java/res/values-v17/styles.xml:20: <style
name="AppBannerOpenButton">
On 2014/02/19 21:52:49, newt wrote:
> you can use style inheritance to here to ensure that AppBannerOpenButton and
> AppBannerInstallButton are the same, except for color. E.g.
> 
>     <style name="AppBannerOpenButton" parent="@style/AppBannerInstallButton">
>         <item name="android:textColor">#737373</item>
>     </style>
> 
> Or define a comment parent and let both styles inherit from that.
> 
> OR... have a single style called "AppBannerButton" and set the color of each
> button independently.

Implemented the last bit.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java
(right):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:63:
private Observer mObserver;
On 2014/02/19 21:52:49, newt wrote:
> For clarity's sake, can you change this to:
> 
>     private AppBannerView.Observer mObserver;
> 
> ?

Done.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:117:
// Store the Drawable's padding.
On 2014/02/19 21:52:49, newt wrote:
> Huh? Which Drawable's padding?

Clarified.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:137:
// Cache the banner dimensions, adjusting margins for drop shadows defined in
the Drawable.
On 2014/02/19 21:52:49, newt wrote:
> still confused :/ ... Which Drawable?

Explained up top.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:159:
setPadding(paddingStart, paddingTop, paddingEnd, paddingBottom);
On 2014/02/19 21:52:49, newt wrote:
> Do you mean:
> 
>     ApiCompatibilityUtils.setPaddingRelative()?
> 
> Or are you assuming the left and right padding are the same (in which case I'd
> just use a single variable called paddingSide)?

Done.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:218:
AppData getAppData() {
On 2014/02/19 21:52:49, newt wrote:
> return a const reference:
> 
>     const AppData& getAppData() const
> 
> (once you add support for that to the JVM)

Added TODO.  (not really)

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:239:
mButtonView.setText(mAppData.installButtonText);
On 2014/02/19 21:52:49, newt wrote:
> Can mAppData.installButtonText have different values? I'd expect it always to
> say "Install", but it seems like that's not the case.

It changes based on the app's price, as well.  They give localized prices for
various markets, I guess.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:249:

On 2014/02/19 21:52:49, newt wrote:
> remove extra line

Done.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:282:

On 2014/02/19 21:52:49, newt wrote:
> I'd explain why you need to override onConfigurationChanged(), and what
> configuration change you're looking out for.

Done.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:297:
while (cannibalized.getChildCount() > 0) {
On 2014/02/19 21:52:49, newt wrote:
> Which child views depend on the screen size? Do you need to replace all the
> child views, or could you just replace one or two of them?

Pretty much all of them.  The font size changes, the padding changes, the icon
height changes, etc.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:407:
Rect bounds = new Rect();
On 2014/02/19 21:52:49, newt wrote:
> I bet android lint will complain about "new Rect()" inside onLayout()

Leftover, deleted.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/banners/RatingView.java
(right):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/RatingView.java:27:
private static Bitmap sStarFull;
On 2014/02/19 21:52:49, newt wrote:
> advantage of statics: If many RatingViews exist simultaneously, they can use
the
> same Bitmaps and hence save memory and decoding time.
> 
> disadvantage of statics: We'll keep this memory around forever, even (in the
> common case) where no more app banners will ever be shown. Plus, findbugs may
> chastise you for thread safety issues here, though that's not an issue IMO.
> 
> I'd lean towards making these member variables instead of statics.

Done.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/RatingView.java:56:
Resources res = getContext().getResources();
On 2014/02/19 21:52:49, newt wrote:
> Simpler:
> 
>     Resources res = getResources();

Done.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/RatingView.java:58:
Bitmap starHalf = BitmapFactory.decodeResource(res,
R.drawable.btn_star_mini_half);
On 2014/02/19 21:52:49, newt wrote:
> suggestion for simplifying this a bit:
> 
>     sStarHalf = BitmapFactory.decodeResource(res,
> R.drawable.btn_star_mini_half);
>     if (!mIsLayoutLTR) {
>         ...
>         sStartHalf = Bitmap.createBitmap(sStarHalf, 0, 0, width, height, m,
> false);
>     }

Problem is then you can't mark the bitmap as final.  Changed it up a bit.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/RatingView.java:77:
int starDimension = canvas.getHeight();
On 2014/02/19 21:52:49, newt wrote:
> Is this the 5th dimension?? Just an opinion, but, "starSize" makes more sense
to
> me.

Yep, shot right past the 4th.

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/banners/SwipableOverlayView.java
(right):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/SwipableOverlayView.java:458:
protected void onViewClicked() {
On 2014/02/19 21:52:49, newt wrote:
> what now?

It's mainly here to be overridden.  Made the class abstract.

https://codereview.chromium.org/141853007/diff/500001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/141853007/diff/500001/chrome/app/generated_re...
chrome/app/generated_resources.grd:15111: +    <if expr="is_android">
On 2014/02/19 21:52:49, newt wrote:
> If these strings are only used in Java, move them to
android_chrome_strings.grd.
> If they live in generated_resources.grd, they get generated as *both* C++ and
> Java strings.

Done.

[David Trainor- moved to gerrit, 2014-02-20 01:22:27]

lgtm

https://chromiumcodereview.appspot.com/141853007/diff/500001/chrome/android/j...
File chrome/android/java/src/org/chromium/chrome/browser/banners/RatingView.java
(right):

https://chromiumcodereview.appspot.com/141853007/diff/500001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/banners/RatingView.java:29:
private static Bitmap sStarEmpty;
Ah I knew it!

[newt (away), 2014-02-20 01:32:22]

one nit, then lgtm

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java
(right):

https://codereview.chromium.org/141853007/diff/500001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerView.java:117:
// Store the Drawable's padding.
On 2014/02/19 23:31:43, dfalcantara wrote:
> On 2014/02/19 21:52:49, newt wrote:
> > Huh? Which Drawable's padding?
> 
> Clarified.

Ah, ha! It makes sense. Danke schoen!

https://codereview.chromium.org/141853007/diff/860001/chrome/android/java/str...
File chrome/android/java/strings/android_chrome_strings.grd (right):

https://codereview.chromium.org/141853007/diff/860001/chrome/android/java/str...
chrome/android/java/strings/android_chrome_strings.grd:280: <message
name="IDS_APP_BANNER_INSTALLING" desc="Button text indicating that an
application is being installed." formatter_data="android_java">
remove formatter_data (same below)

[gone, 2014-02-20 01:56:14]

https://chromiumcodereview.appspot.com/141853007/diff/860001/chrome/android/j...
File chrome/android/java/strings/android_chrome_strings.grd (right):

https://chromiumcodereview.appspot.com/141853007/diff/860001/chrome/android/j...
chrome/android/java/strings/android_chrome_strings.grd:280: <message
name="IDS_APP_BANNER_INSTALLING" desc="Button text indicating that an
application is being installed." formatter_data="android_java">
On 2014/02/20 01:32:22, newt wrote:
> remove formatter_data (same below)

Done.

[gone, 2014-02-20 01:56:21]

The CQ bit was checked by dfalcantara@chromium.org

[commit-bot: I haz the power, 2014-02-20 02:07:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dfalcantara@chromium.org/141853007/900002

[commit-bot: I haz the power, 2014-02-20 05:18:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dfalcantara@chromium.org/141853007/900002

[gone, 2014-02-20 06:04:19]

The CQ bit was checked by dfalcantara@chromium.org

[commit-bot: I haz the power, 2014-02-20 09:18:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dfalcantara@chromium.org/141853007/960002

[commit-bot: I haz the power, 2014-02-20 12:34:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dfalcantara@chromium.org/141853007/960002

[commit-bot: I haz the power, 2014-02-20 12:51:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-20 12:51:19]

Try jobs failed on following builders: mac_chromium_rel

[gone, 2014-02-20 15:35:25]

The CQ bit was checked by dfalcantara@chromium.org

[commit-bot: I haz the power, 2014-02-20 15:38:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dfalcantara@chromium.org/141853007/960002

[commit-bot: I haz the power, 2014-02-20 18:36:54]

Change committed as 252327