Plugs in the new thumbnailing algorithm to ThumbnailTabHelper.

Plugs in the new thumbnailing algorithm to ThumbnailTabHelper.

The new method is available as preview under a command line switch.
Some algorithm fixes remain + a proper Finch experiment.

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

[motek., 2013-05-22 21:43:36]

mazda@: could I trouble you for a review? 

Regards,

motek.

[mazda, 2013-05-23 19:13:57]

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
File chrome/browser/thumbnails/advanced_thumbnail_crop.cc (right):

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/advanced_thumbnail_crop.cc:18: namespace {
nit: add one empty line

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/advanced_thumbnail_crop.cc:29: }
}  // namespace

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/advanced_thumbnail_crop.cc:67:
gfx::ScaleSize(target_size_, max_scale_factor));
Could you share this logic of how the scale factor is chosen with
SimpleThumbnailCrop?

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/advanced_thumbnail_crop.cc:98: "The thumbnail for " <<
context->url << " will not be created.";
Move "<<" from the previous line and align it with the first "<<" on the
previous line.

LOG(WARNING) << "PostSequencedWorkerTask failed. "
             << "The thumbnail for " << context->url << " will not be created.";

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/advanced_thumbnail_crop.cc:170: " will be created the
old-fashioned way.";
LOG(WARNING) << "CreateRetargettedThumbnailImage failed. "
             << "The thumbnail for " << context->url
             << " will be created the old-fashioned way.";

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/advanced_thumbnail_crop.cc:187:
context->score.boring_score *= 1.1f;  // A bit of a boost for retargetted.
Please make a constant variable for 1.1f.

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/advanced_thumbnail_crop.cc:236: 
nit: delete two empty lines

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
File chrome/browser/thumbnails/advanced_thumbnail_crop.h (right):

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/advanced_thumbnail_crop.h:12: class
AdvancedThumbnailCrop : public ThumbnailingAlgorithm {
Can you make the name more descriptive?

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/advanced_thumbnail_crop.h:44: 
nit: delete one empty line.

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
File chrome/browser/thumbnails/advanced_thumbnail_crop_unittest.cc (right):

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/advanced_thumbnail_crop_unittest.cc:17: using
content::BrowserThread;
It looks this is used only once. Why don't you delete this and use
content::BrowserThread directly?

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/advanced_thumbnail_crop_unittest.cc:67:
EXPECT_EQ(clip_result, CLIP_RESULT_SOURCE_SAME_AS_TARGET);
Please write expectation in the first argument of EXPECT_EQ. Same for other
parts.
In this case
EXPECT_EQ(CLIP_RESULT_SOURCE_SAME_AS_TARGET, clip_result)

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/advanced_thumbnail_crop_unittest.cc:69:
EXPECT_EQ(clipping_rect.origin(), gfx::Point(0, 0));
Using ToString makes the test results more readable when it fails.

EXPECT_EX("0,0", clipping_rect.origin().ToString());

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
File chrome/browser/thumbnails/thumbnail_service_impl.h (right):

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/thumbnail_service_impl.h:41: static bool
GetThumbnailRetargettingEnabled();
Because this is used only in .cc, how about hiding this in .cc?
Also I prefer the name IsThumbnailRetargettingEnabled.

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
File chrome/browser/thumbnails/thumbnailing_context.cc (right):

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/thumbnailing_context.cc:8: #include
"content/public/browser/render_widget_host_view.h"
Is this change needed?

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
File chrome/browser/thumbnails/thumbnailing_context.h (right):

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/thumbnailing_context.h:13: #include "ui/base/layout.h"
Is this needed?

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/thumbnailing_context.h:34:
CLIP_RESULT_SOURCE_SAME_AS_TARGET
nit: add "," at the end

https://codereview.chromium.org/15458003/diff/2001/chrome/browser/thumbnails/...
chrome/browser/thumbnails/thumbnailing_context.h:50: ThumbnailingContext() { } 
// Introduced as a test seam only.
I don't think inheritance is necessary.
Why don't you make a function that returns a context for tests?

[motek., 2013-05-27 16:54:54]

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
File chrome/browser/thumbnails/advanced_thumbnail_crop.cc (right):

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/advanced_thumbnail_crop.cc:18: namespace {
On 2013/05/23 19:13:57, mazda wrote:
> nit: add one empty line

Done.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/advanced_thumbnail_crop.cc:29: }
On 2013/05/23 19:13:57, mazda wrote:
> }  // namespace

Done.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/advanced_thumbnail_crop.cc:67:
gfx::ScaleSize(target_size_, max_scale_factor));
On 2013/05/23 19:13:57, mazda wrote:
> Could you share this logic of how the scale factor is chosen with
> SimpleThumbnailCrop?

I did it by applying some refactoring. Effectively, the entire stanza is shared.
I also attempted to clarify comments so that they apply in both contexts.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/advanced_thumbnail_crop.cc:98: "The thumbnail for " <<
context->url << " will not be created.";
On 2013/05/23 19:13:57, mazda wrote:
> Move "<<" from the previous line and align it with the first "<<" on the
> previous line.
> 
> LOG(WARNING) << "PostSequencedWorkerTask failed. "
>              << "The thumbnail for " << context->url << " will not be
created.";

Done.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/advanced_thumbnail_crop.cc:170: " will be created the
old-fashioned way.";
On 2013/05/23 19:13:57, mazda wrote:
> LOG(WARNING) << "CreateRetargettedThumbnailImage failed. "
>              << "The thumbnail for " << context->url
>              << " will be created the old-fashioned way.";

Done.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/advanced_thumbnail_crop.cc:187:
context->score.boring_score *= 1.1f;  // A bit of a boost for retargetted.
On 2013/05/23 19:13:57, mazda wrote:
> Please make a constant variable for 1.1f.

Done.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/advanced_thumbnail_crop.cc:236: 
On 2013/05/23 19:13:57, mazda wrote:
> nit: delete two empty lines

Done.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
File chrome/browser/thumbnails/advanced_thumbnail_crop.h (right):

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/advanced_thumbnail_crop.h:12: class
AdvancedThumbnailCrop : public ThumbnailingAlgorithm {
On 2013/05/23 19:13:57, mazda wrote:
> Can you make the name more descriptive?

Done.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/advanced_thumbnail_crop.h:44: 
On 2013/05/23 19:13:57, mazda wrote:
> nit: delete one empty line.

Done.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
File chrome/browser/thumbnails/advanced_thumbnail_crop_unittest.cc (right):

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/advanced_thumbnail_crop_unittest.cc:17: using
content::BrowserThread;
On 2013/05/23 19:13:57, mazda wrote:
> It looks this is used only once. Why don't you delete this and use
> content::BrowserThread directly?

Done.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/advanced_thumbnail_crop_unittest.cc:67:
EXPECT_EQ(clip_result, CLIP_RESULT_SOURCE_SAME_AS_TARGET);
On 2013/05/23 19:13:57, mazda wrote:
> Please write expectation in the first argument of EXPECT_EQ. Same for other
> parts.
> In this case
> EXPECT_EQ(CLIP_RESULT_SOURCE_SAME_AS_TARGET, clip_result)

Done.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/advanced_thumbnail_crop_unittest.cc:69:
EXPECT_EQ(clipping_rect.origin(), gfx::Point(0, 0));
On 2013/05/23 19:13:57, mazda wrote:
> Using ToString makes the test results more readable when it fails.
> 
> EXPECT_EX("0,0", clipping_rect.origin().ToString());

Done.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
File chrome/browser/thumbnails/thumbnail_service_impl.h (right):

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/thumbnail_service_impl.h:41: static bool
GetThumbnailRetargettingEnabled();
On 2013/05/23 19:13:57, mazda wrote:
> Because this is used only in .cc, how about hiding this in .cc?
> Also I prefer the name IsThumbnailRetargettingEnabled.

Done.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
File chrome/browser/thumbnails/thumbnailing_context.cc (right):

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/thumbnailing_context.cc:8: #include
"content/public/browser/render_widget_host_view.h"
On 2013/05/23 19:13:57, mazda wrote:
> Is this change needed?

No, a leftover of something I tried out. Un-done.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
File chrome/browser/thumbnails/thumbnailing_context.h (right):

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/thumbnailing_context.h:13: #include "ui/base/layout.h"
On 2013/05/23 19:13:57, mazda wrote:
> Is this needed?

layout? Not anymore, my bad. Sorry for the sloppiness, I should have noticed
that.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/thumbnailing_context.h:34:
CLIP_RESULT_SOURCE_SAME_AS_TARGET
On 2013/05/23 19:13:57, mazda wrote:
> nit: add "," at the end

Done.

https://chromiumcodereview.appspot.com/15458003/diff/2001/chrome/browser/thum...
chrome/browser/thumbnails/thumbnailing_context.h:50: ThumbnailingContext() { } 
// Introduced as a test seam only.
On 2013/05/23 19:13:57, mazda wrote:
> I don't think inheritance is necessary.
> Why don't you make a function that returns a context for tests?

Good idea. Done.

[mazda, 2013-05-29 21:50:27]

Sorry for the delay.
LGTM with a few nits.

https://codereview.chromium.org/15458003/diff/34001/chrome/browser/thumbnails...
File chrome/browser/thumbnails/content_based_thumbnailing_algorithm.cc (right):

https://codereview.chromium.org/15458003/diff/34001/chrome/browser/thumbnails...
chrome/browser/thumbnails/content_based_thumbnailing_algorithm.cc:108: 
nit: remove the empty line

https://codereview.chromium.org/15458003/diff/34001/chrome/browser/thumbnails...
File chrome/browser/thumbnails/content_based_thumbnailing_algorithm.h (right):

https://codereview.chromium.org/15458003/diff/34001/chrome/browser/thumbnails...
chrome/browser/thumbnails/content_based_thumbnailing_algorithm.h:11: 
Could you add a brief comment?

https://codereview.chromium.org/15458003/diff/34001/chrome/browser/thumbnails...
File chrome/browser/thumbnails/content_based_thumbnailing_algorithm_unittest.cc
(right):

https://codereview.chromium.org/15458003/diff/34001/chrome/browser/thumbnails...
chrome/browser/thumbnails/content_based_thumbnailing_algorithm_unittest.cc:31:
clip_result_ = context.clip_result;
This is unnecessary.

https://codereview.chromium.org/15458003/diff/34001/chrome/browser/thumbnails...
chrome/browser/thumbnails/content_based_thumbnailing_algorithm_unittest.cc:34:
SkBitmap captured_bitmap_;
Please make these variables private and provide accessors.

https://codereview.chromium.org/15458003/diff/34001/chrome/browser/thumbnails...
File chrome/browser/thumbnails/simple_thumbnail_crop_unittest.cc (right):

https://codereview.chromium.org/15458003/diff/34001/chrome/browser/thumbnails...
chrome/browser/thumbnails/simple_thumbnail_crop_unittest.cc:206:
EXPECT_EQ(clip_result, thumbnails::CLIP_RESULT_SOURCE_IS_SMALLER);
The expectation is first.
Same applies to all the others.

[motek., 2013-05-30 15:00:03]

https://chromiumcodereview.appspot.com/15458003/diff/34001/chrome/browser/thu...
File chrome/browser/thumbnails/content_based_thumbnailing_algorithm.cc (right):

https://chromiumcodereview.appspot.com/15458003/diff/34001/chrome/browser/thu...
chrome/browser/thumbnails/content_based_thumbnailing_algorithm.cc:108: 
On 2013/05/29 21:50:27, mazda wrote:
> nit: remove the empty line

Done.

https://chromiumcodereview.appspot.com/15458003/diff/34001/chrome/browser/thu...
File chrome/browser/thumbnails/content_based_thumbnailing_algorithm.h (right):

https://chromiumcodereview.appspot.com/15458003/diff/34001/chrome/browser/thu...
chrome/browser/thumbnails/content_based_thumbnailing_algorithm.h:11: 
On 2013/05/29 21:50:27, mazda wrote:
> Could you add a brief comment?

Done.

https://chromiumcodereview.appspot.com/15458003/diff/34001/chrome/browser/thu...
File chrome/browser/thumbnails/content_based_thumbnailing_algorithm_unittest.cc
(right):

https://chromiumcodereview.appspot.com/15458003/diff/34001/chrome/browser/thu...
chrome/browser/thumbnails/content_based_thumbnailing_algorithm_unittest.cc:31:
clip_result_ = context.clip_result;
On 2013/05/29 21:50:27, mazda wrote:
> This is unnecessary.

Done.

https://chromiumcodereview.appspot.com/15458003/diff/34001/chrome/browser/thu...
chrome/browser/thumbnails/content_based_thumbnailing_algorithm_unittest.cc:34:
SkBitmap captured_bitmap_;
On 2013/05/29 21:50:27, mazda wrote:
> Please make these variables private and provide accessors.

Done.

https://chromiumcodereview.appspot.com/15458003/diff/34001/chrome/browser/thu...
File chrome/browser/thumbnails/simple_thumbnail_crop_unittest.cc (right):

https://chromiumcodereview.appspot.com/15458003/diff/34001/chrome/browser/thu...
chrome/browser/thumbnails/simple_thumbnail_crop_unittest.cc:206:
EXPECT_EQ(clip_result, thumbnails::CLIP_RESULT_SOURCE_IS_SMALLER);
On 2013/05/29 21:50:27, mazda wrote:
> The expectation is first.
> Same applies to all the others.

Done.

[motek., 2013-05-30 15:05:39]

thakis@: OWNERS'
The CL has been reviewed for content (mazda@) already but owner's review of the
entire thing is still due. 

The feature will be accessible under flag as a preview only for now. Another CL
will turn it into a v. limited Finch experiment.

[Nico, 2013-05-30 18:19:17]

That's a lot of code for computing thumbnails.

Are there any thumbnail computation speed (== battery usage) comparisons
anywhere?

Who should be in chrome/browser/thumbnails/OWNERS?

https://chromiumcodereview.appspot.com/15458003/diff/49001/chrome/browser/thu...
File chrome/browser/thumbnails/content_based_thumbnailing_algorithm.cc (right):

https://chromiumcodereview.appspot.com/15458003/diff/49001/chrome/browser/thu...
chrome/browser/thumbnails/content_based_thumbnailing_algorithm.cc:20: static
const char kThumbnailHistogramName[] = "Thumbnail.RetargetMS";
const has implicit internal linkage, so you don't need the "static". Since this
is in an unnamed namespace, you wouldn't even need it if that wasn't the case.

https://chromiumcodereview.appspot.com/15458003/diff/49001/chrome/common/chro...
File chrome/common/chrome_switches.cc (right):

https://chromiumcodereview.appspot.com/15458003/diff/49001/chrome/common/chro...
chrome/common/chrome_switches.cc:676: const char kEnableThumbnailRetargetting[] 
 = "enable-thumbnail-retargetting";
"Retargeting" with just one "t" I think?

[motek., 2013-05-30 20:25:01]

On 2013/05/30 18:19:17, Nico wrote:
> That's a lot of code for computing thumbnails.
It is, but the idea itself is rather complex, too. This may or may not pan out,
but we tested this on a limited subset of screenshots (with human scorers) and
it looked promising. 

> 
> Are there any thumbnail computation speed (== battery usage) comparisons
> anywhere?
Not yet, I think. Generally this will be a concern so I it will need to be
captured somehow in the upcoming field trial. I am working on that, it will
absolutely not be enabled to all without proper trailing. 

> 
> Who should be in chrome/browser/thumbnails/OWNERS?
There isn't one now, I guess there was no need. I do not know who should be
there. The mechanism has been originally put in place by brettw@, but that was
over 3 years ago...

[motek., 2013-05-30 20:25:09]

https://chromiumcodereview.appspot.com/15458003/diff/49001/chrome/browser/thu...
File chrome/browser/thumbnails/content_based_thumbnailing_algorithm.cc (right):

https://chromiumcodereview.appspot.com/15458003/diff/49001/chrome/browser/thu...
chrome/browser/thumbnails/content_based_thumbnailing_algorithm.cc:20: static
const char kThumbnailHistogramName[] = "Thumbnail.RetargetMS";
On 2013/05/30 18:19:17, Nico wrote:
> const has implicit internal linkage, so you don't need the "static". Since
this
> is in an unnamed namespace, you wouldn't even need it if that wasn't the case.

Done.

https://chromiumcodereview.appspot.com/15458003/diff/49001/chrome/common/chro...
File chrome/common/chrome_switches.cc (right):

https://chromiumcodereview.appspot.com/15458003/diff/49001/chrome/common/chro...
chrome/common/chrome_switches.cc:676: const char kEnableThumbnailRetargetting[] 
 = "enable-thumbnail-retargetting";
On 2013/05/30 18:19:17, Nico wrote:
> "Retargeting" with just one "t" I think?

Depends on your version of English, I suppose. Anyway, changed to the US
spelling everywhere as it is more common in the codebase.

[Nico, 2013-05-31 21:22:44]

On Thu, May 30, 2013 at 1:25 PM, <motek@chromium.org> wrote:

> On 2013/05/30 18:19:17, Nico wrote:
>
>> That's a lot of code for computing thumbnails.
>>
> It is, but the idea itself is rather complex, too. This may or may not pan
> out,
> but we tested this on a limited subset of screenshots (with human scorers)
> and
> it looked promising.
>
>
>
>  Are there any thumbnail computation speed (== battery usage) comparisons
>> anywhere?
>>
> Not yet, I think. Generally this will be a concern so I it will need to be
> captured somehow in the upcoming field trial. I am working on that, it will
> absolutely not be enabled to all without proper trailing.


Cool. Should there be a tracking bug for that, blocking bug 155269?


> Who should be in chrome/browser/thumbnails/**OWNERS?
> There isn't one now, I guess there was no need. I do not know who should be
> there. The mechanism has been originally put in place by brettw@, but
> that was
> over 3 years ago...
>

Can you find someone to own this code with you? (Maybe mazda?)


>
>
>
>
https://chromiumcodereview.**appspot.com/15458003/<https://chromiumcodereview...
>

[Nico, 2013-05-31 21:25:05]

This CL LGTM.

[motek., 2013-06-03 15:10:16]

On 2013/05/31 21:22:44, Nico wrote:
> On Thu, May 30, 2013 at 1:25 PM, <mailto:motek@chromium.org> wrote:
> 
> > On 2013/05/30 18:19:17, Nico wrote:
> >
> >> That's a lot of code for computing thumbnails.
> >>
> > It is, but the idea itself is rather complex, too. This may or may not pan
> > out,
> > but we tested this on a limited subset of screenshots (with human scorers)
> > and
> > it looked promising.
> >
> >
> >
> >  Are there any thumbnail computation speed (== battery usage) comparisons
> >> anywhere?
> >>
> > Not yet, I think. Generally this will be a concern so I it will need to be
> > captured somehow in the upcoming field trial. I am working on that, it will
> > absolutely not be enabled to all without proper trailing.
> 
> 
> Cool. Should there be a tracking bug for that, blocking bug 155269?
Done so. The number is 245455.

> 
> 
> > Who should be in chrome/browser/thumbnails/**OWNERS?
> > There isn't one now, I guess there was no need. I do not know who should be
> > there. The mechanism has been originally put in place by brettw@, but
> > that was
> > over 3 years ago...
> >
> 
> Can you find someone to own this code with you? (Maybe mazda?)
I suppose this could be a good idea. But I am but a 'lowly provisional
committer', so I think I would need to get myself to a full committer first. 

> 
> 
> >
> >
> >
> >
>
https://chromiumcodereview.**appspot.com/15458003/%3Chttps://chromiumcoderevi...>
> >

[commit-bot: I haz the power, 2013-06-03 15:11:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/motek@chromium.org/15458003/62003

[commit-bot: I haz the power, 2013-06-03 15:30:36]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-06-03 17:36:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/motek@chromium.org/15458003/74003

[commit-bot: I haz the power, 2013-06-04 06:04:11]

Change committed as 203856