Add FontRenderParamsQuery.

Add FontRenderParamsQuery.

Add a new struct that encapsulates the parameters used to
initialize FontRenderParams.

Also remove all the existing FontRenderParams functions in
favor of a single GetFontRenderParams() function that takes
a FontRenderParamsQuery.

BUG=395313, 396659
TBR=boliu@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285425

[Daniel Erat, 2014-07-23 20:40:35]

i still need to test this on non-linux platforms, but i figured i'd check how
you feel about the overall approach. it simplifies the non-linux implementations
of FontRenderParams and seems cleaner to me from the caller's perspective.

https://codereview.chromium.org/413003002/diff/1/android_webview/native/aw_se...
File android_webview/native/aw_settings.cc (right):

https://codereview.chromium.org/413003002/diff/1/android_webview/native/aw_se...
android_webview/native/aw_settings.cc:38: CR_DEFINE_STATIC_LOCAL(const
gfx::FontRenderParams, params,
needed to switch to this to avoid clang compilation errors. this macro allows us
to intentionally leak local static variables; clang warns about the
shutdown-time cost of destroying them otherwise

https://codereview.chromium.org/413003002/diff/1/ui/gfx/font_render_params.h
File ui/gfx/font_render_params.h (right):

https://codereview.chromium.org/413003002/diff/1/ui/gfx/font_render_params.h#...
ui/gfx/font_render_params.h:83: // gfx::Font::FontStyle bit field, or -1 if
unset.
i don't love the -1 default here, because someone may do this and then be
confused about why their style is ignored:

  FontRenderParamsQuery query;
  query.style |= Font::BOLD;
  FontRenderParams params = GetFontRenderParams(query, NULL);

other alternatives that i also don't love:

- initializing this to gfx::Font::NORMAL, which means that there's no way to
signal that we don't want to include a style in the fontconfig query

- adding a separate use_style member, which someone can forget to set

- turning this into a class, making this a private member, and adding a
set_style() method so it's impossible to do a bitwise or with the "unset" value

can you suggest anything better?

https://codereview.chromium.org/413003002/diff/40001/android_webview/native/a...
File android_webview/native/aw_settings.cc (right):

https://codereview.chromium.org/413003002/diff/40001/android_webview/native/a...
android_webview/native/aw_settings.cc:38: CR_DEFINE_STATIC_LOCAL(const
gfx::FontRenderParams, params,
needed to switch to this to avoid clang compilation errors. this macro allows us
to intentionally leak local static variables; clang warns about the
shutdown-time cost of destroying them otherwise

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_params.h
File ui/gfx/font_render_params.h (right):

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:62: // A query used to determine the appropriate
FontRenderParams.
if we want to, this should also make it easy to cache FontRenderParams in the
linux implementation to avoid issuing a bunch of redundant fontconfig queries

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:64: explicit FontRenderParamsQuery(bool
for_web_contents);
the for_web_contents argument here is a bit strange (along with it not being
checked in is_empty()), but it allows inline calls to initialize
FontRenderParams static variables and should disappear after i do
http://crbug.com/396659

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:83: // gfx::Font::FontStyle bit field, or -1 if
unset.
i don't love the -1 default here, because someone may do this and then be
confused about why their style is ignored:

  FontRenderParamsQuery query;
  query.style |= Font::BOLD;
  FontRenderParams params = GetFontRenderParams(query, NULL);

other alternatives that i also don't love:

- initializing this to gfx::Font::NORMAL, which means that there's no way to
signal that we don't want to include a style in the fontconfig query

- adding a separate use_style member, which someone can forget to set

- turning this into a class, making this a private member, and adding a
set_style() method so it's impossible to do a bitwise or with the "unset" value

can you think of anything better?

[Daniel Erat, 2014-07-23 20:42:45]

(sorry, please disregard duplicate comments i forgot to delete from patch set 1.
i still don't have a workflow that fits well with me fixing minor issues that i
notice when doing a once-over in the issue tracker before mailing the change
:-/)

[msw, 2014-07-23 22:16:35]

https://codereview.chromium.org/413003002/diff/40001/android_webview/native/a...
File android_webview/native/aw_settings.cc (right):

https://codereview.chromium.org/413003002/diff/40001/android_webview/native/a...
android_webview/native/aw_settings.cc:38: CR_DEFINE_STATIC_LOCAL(const
gfx::FontRenderParams, params,
On 2014/07/23 20:40:34, Daniel Erat wrote:
> needed to switch to this to avoid clang compilation errors. this macro allows
us
> to intentionally leak local static variables; clang warns about the
> shutdown-time cost of destroying them otherwise

Maybe use LazyInstance, just because they're far more common?

https://codereview.chromium.org/413003002/diff/40001/android_webview/native/a...
android_webview/native/aw_settings.cc:39:
(gfx::GetFontRenderParams(gfx::FontRenderParamsQuery(true), NULL)));
nit: remove extra parens?

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_params.h
File ui/gfx/font_render_params.h (right):

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:57: // TODO(derat): Remove this; we don't set it in
the browser and mostly ignore
I think you mean to put this with |subpixel_positioning|, not
|subpixel_rendering|, which seems far more important and widely used.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:64: explicit FontRenderParamsQuery(bool
for_web_contents);
On 2014/07/23 20:40:34, Daniel Erat wrote:
> the for_web_contents argument here is a bit strange (along with it not being
> checked in is_empty()), but it allows inline calls to initialize
> FontRenderParams static variables and should disappear after i do
> http://crbug.com/396659

Acknowledged.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:72: // TODO(derat): Remove this once
FontRenderParams::subpixel_rendering is gone:
Do you mean |subpixel_positioning|?

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:83: // gfx::Font::FontStyle bit field, or -1 if
unset.
On 2014/07/23 20:40:34, Daniel Erat wrote:
> i don't love the -1 default here, because someone may do this and then be
> confused about why their style is ignored:
> 
>   FontRenderParamsQuery query;
>   query.style |= Font::BOLD;
>   FontRenderParams params = GetFontRenderParams(query, NULL);
> 
> other alternatives that i also don't love:
> 
> - initializing this to gfx::Font::NORMAL, which means that there's no way to
> signal that we don't want to include a style in the fontconfig query
> 
> - adding a separate use_style member, which someone can forget to set
> 
> - turning this into a class, making this a private member, and adding a
> set_style() method so it's impossible to do a bitwise or with the "unset"
value
> 
> can you think of anything better?

Hmmm, I agree it's not ideal, but the results of misuse should be apparent and
without really severe consequences. I think it's fine as-is, plus maybe adding a
gfx::Font enum -1 like NONE, UNKNOWN_STYLE, UNSET, or similar.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:92: std::string* family_out);
Should |family_out| be a member of FontRenderParams?
(maybe done in a separate CL)

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
File ui/gfx/font_render_params_android.cc (right):

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params_android.cc:35: if (!query.is_empty() || family_out)
Add a short explanatory comment here, like "// Customized font rendering
settings are not supported, only defaults."

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
File ui/gfx/font_render_params_linux_unittest.cc (right):

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params_linux_unittest.cc:281: FontRenderParams params =
GetFontRenderParams(query, &suggested_family);
nit: I just noticed that this is the only test that attempts to get the
suggested family via |family_out|, and it's a test where we don't get a match
and fall back to the first family of the list supplied in the query. Perhaps add
a test that gets the |family_out| with a matching config? (later is ok)

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
File ui/gfx/font_render_params_win.cc (right):

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params_win.cc:77: if (!query.is_empty() || family_out)
ditto (needs comment)

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
File ui/gfx/platform_font_pango.cc (right):

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
ui/gfx/platform_font_pango.cc:109: // TODO(davemoore) What should we do about
other weights? We currently only
nit: one-liner "TODO(davemoore) Support weights other than BOLD?"

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
ui/gfx/platform_font_pango.cc:118: const FontRenderParams params =
gfx::GetFontRenderParams(query, &font_family);
nit: maybe inline this below?

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
ui/gfx/platform_font_pango.cc:129: query.style = gfx::Font::NORMAL;
Should this omit the style from the query if it's unknown, or is it right to
assume that the caller requires a 'normal' style font? Either seems reasonable
to me, I'm just asking. (you may have asked a related question in an earlier
patch set, but I can't find it with a quick search)

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
ui/gfx/platform_font_pango.cc:130: const FontRenderParams params =
gfx::GetFontRenderParams(query, NULL);
nit: maybe inline this below?

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
ui/gfx/platform_font_pango.cc:177: const FontRenderParams render_params =
gfx::GetFontRenderParams(query, NULL);
nit: maybe inline this below?

[Daniel Erat, 2014-07-23 22:47:18]

+jochen for tiny chrome/ and content/ changes

https://codereview.chromium.org/413003002/diff/40001/android_webview/native/a...
File android_webview/native/aw_settings.cc (right):

https://codereview.chromium.org/413003002/diff/40001/android_webview/native/a...
android_webview/native/aw_settings.cc:38: CR_DEFINE_STATIC_LOCAL(const
gfx::FontRenderParams, params,
On 2014/07/23 22:16:34, msw wrote:
> On 2014/07/23 20:40:34, Daniel Erat wrote:
> > needed to switch to this to avoid clang compilation errors. this macro
allows
> us
> > to intentionally leak local static variables; clang warns about the
> > shutdown-time cost of destroying them otherwise
> 
> Maybe use LazyInstance, just because they're far more common?

hmm, the header makes it sound like it's mostly there for when thread-safety is
needed, and that function-local statics are preferable otherwise. does
LazyInstance work for objects that need more complicated initialization than
just calling an argument-less c'tor?

https://codereview.chromium.org/413003002/diff/40001/android_webview/native/a...
android_webview/native/aw_settings.cc:39:
(gfx::GetFontRenderParams(gfx::FontRenderParamsQuery(true), NULL)));
On 2014/07/23 22:16:34, msw wrote:
> nit: remove extra parens?

these actually seemed to be required by the macro

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_params.h
File ui/gfx/font_render_params.h (right):

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:57: // TODO(derat): Remove this; we don't set it in
the browser and mostly ignore
On 2014/07/23 22:16:34, msw wrote:
> I think you mean to put this with |subpixel_positioning|, not
> |subpixel_rendering|, which seems far more important and widely used.

whoops, yeah. done

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:72: // TODO(derat): Remove this once
FontRenderParams::subpixel_rendering is gone:
On 2014/07/23 22:16:35, msw wrote:
> Do you mean |subpixel_positioning|?

sigh, yep. at least i am consistently wrong.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:83: // gfx::Font::FontStyle bit field, or -1 if
unset.
On 2014/07/23 22:16:35, msw wrote:
> On 2014/07/23 20:40:34, Daniel Erat wrote:
> > i don't love the -1 default here, because someone may do this and then be
> > confused about why their style is ignored:
> > 
> >   FontRenderParamsQuery query;
> >   query.style |= Font::BOLD;
> >   FontRenderParams params = GetFontRenderParams(query, NULL);
> > 
> > other alternatives that i also don't love:
> > 
> > - initializing this to gfx::Font::NORMAL, which means that there's no way to
> > signal that we don't want to include a style in the fontconfig query
> > 
> > - adding a separate use_style member, which someone can forget to set
> > 
> > - turning this into a class, making this a private member, and adding a
> > set_style() method so it's impossible to do a bitwise or with the "unset"
> value
> > 
> > can you think of anything better?
> 
> Hmmm, I agree it's not ideal, but the results of misuse should be apparent and
> without really severe consequences. I think it's fine as-is, plus maybe adding
a
> gfx::Font enum -1 like NONE, UNKNOWN_STYLE, UNSET, or similar.

i'm a bit nervous about adding a -1 constant to gfx::Font, since some other
class that uses Font::Style may not handle NONE correctly and instead believe
that all of the style bits are set. i'd rather keep the -1 hack constrained to
FontRenderParams.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:92: std::string* family_out);
On 2014/07/23 22:16:35, msw wrote:
> Should |family_out| be a member of FontRenderParams?
> (maybe done in a separate CL)

hmm, i don't think so. it's really not very similar to the other members of that
struct, and i mostly just put it here to avoid needing to do an extra fontconfig
query. this is probably something we should figure out if/when trying to unify
the fallback code between blink and the browser for http://crbug.com/395313.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
File ui/gfx/font_render_params_android.cc (right):

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params_android.cc:35: if (!query.is_empty() || family_out)
On 2014/07/23 22:16:35, msw wrote:
> Add a short explanatory comment here, like "// Customized font rendering
> settings are not supported, only defaults."

Done.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
File ui/gfx/font_render_params_linux_unittest.cc (right):

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params_linux_unittest.cc:281: FontRenderParams params =
GetFontRenderParams(query, &suggested_family);
On 2014/07/23 22:16:35, msw wrote:
> nit: I just noticed that this is the only test that attempts to get the
> suggested family via |family_out|, and it's a test where we don't get a match
> and fall back to the first family of the list supplied in the query. Perhaps
add
> a test that gets the |family_out| with a matching config? (later is ok)

sure, was planning to do this to check that we're actually using the font that
fontconfig instructs us to

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
File ui/gfx/font_render_params_win.cc (right):

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params_win.cc:77: if (!query.is_empty() || family_out)
On 2014/07/23 22:16:35, msw wrote:
> ditto (needs comment)

Done.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
File ui/gfx/platform_font_pango.cc (right):

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
ui/gfx/platform_font_pango.cc:109: // TODO(davemoore) What should we do about
other weights? We currently only
On 2014/07/23 22:16:35, msw wrote:
> nit: one-liner "TODO(davemoore) Support weights other than BOLD?"

Done.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
ui/gfx/platform_font_pango.cc:118: const FontRenderParams params =
gfx::GetFontRenderParams(query, &font_family);
On 2014/07/23 22:16:35, msw wrote:
> nit: maybe inline this below?

seems scary since it updates |font_family|, which is another argument to
InitFromDetails (we're passing a reference, so i think it'd be okay, but
still...)

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
ui/gfx/platform_font_pango.cc:129: query.style = gfx::Font::NORMAL;
On 2014/07/23 22:16:35, msw wrote:
> Should this omit the style from the query if it's unknown, or is it right to
> assume that the caller requires a 'normal' style font? Either seems reasonable
> to me, I'm just asking. (you may have asked a related question in an earlier
> patch set, but I can't find it with a quick search)

i think that requesting normal here is the right thing to do, since that's what
gfx::Font is using and how it'll be rendered. the not-including-style-in-query
thing is really just useful when getting defaults, in my mind.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
ui/gfx/platform_font_pango.cc:130: const FontRenderParams params =
gfx::GetFontRenderParams(query, NULL);
On 2014/07/23 22:16:35, msw wrote:
> nit: maybe inline this below?

Done.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
ui/gfx/platform_font_pango.cc:177: const FontRenderParams render_params =
gfx::GetFontRenderParams(query, NULL);
On 2014/07/23 22:16:35, msw wrote:
> nit: maybe inline this below?

Done.

[msw, 2014-07-23 22:54:31]

lgtm

https://codereview.chromium.org/413003002/diff/40001/android_webview/native/a...
File android_webview/native/aw_settings.cc (right):

https://codereview.chromium.org/413003002/diff/40001/android_webview/native/a...
android_webview/native/aw_settings.cc:38: CR_DEFINE_STATIC_LOCAL(const
gfx::FontRenderParams, params,
On 2014/07/23 22:47:17, Daniel Erat wrote:
> On 2014/07/23 22:16:34, msw wrote:
> > On 2014/07/23 20:40:34, Daniel Erat wrote:
> > > needed to switch to this to avoid clang compilation errors. this macro
> allows
> > us
> > > to intentionally leak local static variables; clang warns about the
> > > shutdown-time cost of destroying them otherwise
> > 
> > Maybe use LazyInstance, just because they're far more common?
> 
> hmm, the header makes it sound like it's mostly there for when thread-safety
is
> needed, and that function-local statics are preferable otherwise. does
> LazyInstance work for objects that need more complicated initialization than
> just calling an argument-less c'tor?

Okay, CR_DEFINE_STATIC_LOCAL seems fine, but you may want to ping someone more
familiar with its use to take a quick look.

https://codereview.chromium.org/413003002/diff/40001/android_webview/native/a...
android_webview/native/aw_settings.cc:39:
(gfx::GetFontRenderParams(gfx::FontRenderParamsQuery(true), NULL)));
On 2014/07/23 22:47:17, Daniel Erat wrote:
> On 2014/07/23 22:16:34, msw wrote:
> > nit: remove extra parens?
> 
> these actually seemed to be required by the macro

Acknowledged.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_params.h
File ui/gfx/font_render_params.h (right):

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:62: // A query used to determine the appropriate
FontRenderParams.
On 2014/07/23 20:40:34, Daniel Erat wrote:
> if we want to, this should also make it easy to cache FontRenderParams in the
> linux implementation to avoid issuing a bunch of redundant fontconfig queries

That'd be good, I think we do a bunch on startup...

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:83: // gfx::Font::FontStyle bit field, or -1 if
unset.
On 2014/07/23 22:47:17, Daniel Erat wrote:
> On 2014/07/23 22:16:35, msw wrote:
> > On 2014/07/23 20:40:34, Daniel Erat wrote:
> > > i don't love the -1 default here, because someone may do this and then be
> > > confused about why their style is ignored:
> > > 
> > >   FontRenderParamsQuery query;
> > >   query.style |= Font::BOLD;
> > >   FontRenderParams params = GetFontRenderParams(query, NULL);
> > > 
> > > other alternatives that i also don't love:
> > > 
> > > - initializing this to gfx::Font::NORMAL, which means that there's no way
to
> > > signal that we don't want to include a style in the fontconfig query
> > > 
> > > - adding a separate use_style member, which someone can forget to set
> > > 
> > > - turning this into a class, making this a private member, and adding a
> > > set_style() method so it's impossible to do a bitwise or with the "unset"
> > value
> > > 
> > > can you think of anything better?
> > 
> > Hmmm, I agree it's not ideal, but the results of misuse should be apparent
and
> > without really severe consequences. I think it's fine as-is, plus maybe
adding
> a
> > gfx::Font enum -1 like NONE, UNKNOWN_STYLE, UNSET, or similar.
> 
> i'm a bit nervous about adding a -1 constant to gfx::Font, since some other
> class that uses Font::Style may not handle NONE correctly and instead believe
> that all of the style bits are set. i'd rather keep the -1 hack constrained to
> FontRenderParams.

Acknowledged.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/font_render_param...
ui/gfx/font_render_params.h:92: std::string* family_out);
On 2014/07/23 22:47:18, Daniel Erat wrote:
> On 2014/07/23 22:16:35, msw wrote:
> > Should |family_out| be a member of FontRenderParams?
> > (maybe done in a separate CL)
> 
> hmm, i don't think so. it's really not very similar to the other members of
that
> struct, and i mostly just put it here to avoid needing to do an extra
fontconfig
> query. this is probably something we should figure out if/when trying to unify
> the fallback code between blink and the browser for http://crbug.com/395313.

Acknowledged.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
File ui/gfx/platform_font_pango.cc (right):

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
ui/gfx/platform_font_pango.cc:118: const FontRenderParams params =
gfx::GetFontRenderParams(query, &font_family);
On 2014/07/23 22:47:18, Daniel Erat wrote:
> On 2014/07/23 22:16:35, msw wrote:
> > nit: maybe inline this below?
> 
> seems scary since it updates |font_family|, which is another argument to
> InitFromDetails (we're passing a reference, so i think it'd be okay, but
> still...)

Acknowledged.

https://codereview.chromium.org/413003002/diff/40001/ui/gfx/platform_font_pan...
ui/gfx/platform_font_pango.cc:129: query.style = gfx::Font::NORMAL;
On 2014/07/23 22:47:18, Daniel Erat wrote:
> On 2014/07/23 22:16:35, msw wrote:
> > Should this omit the style from the query if it's unknown, or is it right to
> > assume that the caller requires a 'normal' style font? Either seems
reasonable
> > to me, I'm just asking. (you may have asked a related question in an earlier
> > patch set, but I can't find it with a quick search)
> 
> i think that requesting normal here is the right thing to do, since that's
what
> gfx::Font is using and how it'll be rendered. the not-including-style-in-query
> thing is really just useful when getting defaults, in my mind.

Acknowledged.

[jochen (gone - plz use gerrit), 2014-07-24 09:01:46]

lgtm

[Daniel Erat, 2014-07-24 19:02:42]

The CQ bit was checked by derat@chromium.org

[Daniel Erat, 2014-07-24 19:03:14]

TBR boliu@ for tiny android change

[commit-bot: I haz the power, 2014-07-24 19:04:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/derat@chromium.org/413003002/120001

[boliu, 2014-07-24 19:04:25]

On 2014/07/24 19:03:14, Daniel Erat wrote:
> TBR boliu@ for tiny android change

lgtm

[commit-bot: I haz the power, 2014-07-24 22:52:49]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_rel_swarming on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel_sw...)

[commit-bot: I haz the power, 2014-07-25 00:10:41]

Change committed as 285425