Creating PNG images from web content to be used by OverviewMode navigation

Creating PNG images from web content to be used by OverviewMode navigation

This patch reads back content from the GPU in quarter res and stores it (in memory) as PNG.
The OverviewMode can then grab an image through an activity to present it accordingly.

Since we do not have browser test framework for Athena, there is no unit test yet.
I took however issue 412474 on to mitigate that and I will use this test as the gunieapig test.

In the meantime sadrul@chromium.org can start using this for his overviewmode presentation development.

BUG=408837
TEST=Visual: Added Logging statements to see that the code gets properly executed.

Committed: https://crrev.com/7144431a072fad80fc82ee87e7216011c534ef3b
Cr-Commit-Position: refs/heads/master@{#296729}

[Mr4D (OOO till 08-26), 2014-09-19 22:17:16]

skuhne@chromium.org changed reviewers:
+ oshima@chromium.org, sadrul@chromium.org

[Mr4D (OOO till 08-26), 2014-09-19 22:17:17]

Please have a look!

This is as discussed adding the possibility to retrieve images for activities to
be displayed in overview mode.

I thought about adding a cache for decoded images in the activities, but I think
that such a cache should rather be added on the overview mode side.

[oshima, 2014-09-22 16:10:29]

https://codereview.chromium.org/591693002/diff/1/athena/activity/public/activ...
File athena/activity/public/activity_view_model.h (right):

https://codereview.chromium.org/591693002/diff/1/athena/activity/public/activ...
athena/activity/public/activity_view_model.h:11: #include "ui/gfx/image/image.h"
forward decl should be enough

https://codereview.chromium.org/591693002/diff/1/athena/activity/public/activ...
athena/activity/public/activity_view_model.h:61: virtual scoped_ptr<gfx::Image>
GetOverviewModeImage() = 0;
Is there particular reason why this needs to be scoped_ptr?
The image data in gfx::Image is ref counted.

https://codereview.chromium.org/591693002/diff/1/athena/content/app_activity.cc
File athena/content/app_activity.cc (right):

https://codereview.chromium.org/591693002/diff/1/athena/content/app_activity....
athena/content/app_activity.cc:30: content_proxy_->ContentGetsDestroyed();
At this point, is AppAcitivyt's content supposed to be destroyed?

If so, can we add DCHECK?

https://codereview.chromium.org/591693002/diff/1/athena/content/app_activity.h
File athena/content/app_activity.h (right):

https://codereview.chromium.org/591693002/diff/1/athena/content/app_activity....
athena/content/app_activity.h:31: // Gets the content proxy so that the AppProxy
can take it over.
Does AppProxy mean AppActivityProxy?

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy.cc
File athena/content/content_proxy.cc (right):

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:24: class ProxyImageData : public
base::RefCountedThreadSafe<ProxyImageData> {
If this is to be used by overview mode, it will be decoded again.
Why this is encoded where? Is it enough to encode when saving to secondary
storage?

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:36: callback.Run();
should this be DCHECK? Does it make sense to run callback when the task wasn't
posed?

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:81: if (image_.IsEmpty())
shouldn't this be !image_.IsEmpty(), or
maybe checking png_data is better/ (if both are empty, it will return empty
image)

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:86:
image_reps.push_back(gfx::ImagePNGRep(png_data_, 2.0f));
why this is using 2x scale factor?

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:106: if (web_view_ && !content_visible_) {
don't you have to change content_visible_? 

Rather than having another flag, is it possible to use the web_view's visibility
instead?

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy.h
File athena/content/content_proxy.h (right):

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.h:53: scoped_ptr<gfx::Image> GetContentImage();
ditto

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.h:55: // The content is about to get destroyed by
its creator.
Called when the content is about to ...

If this is called before the content is destroyed, it should be
something like

OnContenteBeingDestroyed or
OnPreContentGetsDestroyed

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.h:72: bool GetContentAsImage();
CreateContentImage ?

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.h:75: void ContentReadBackAsImage(bool success,
const SkBitmap& bitmap);
OnContentImage{Read/Created/..}?

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.h:81: // ContentGetsDestroyed() gets called and
then set to NULL.
can you document ownership?

[sadrul, 2014-09-22 16:13:33]

https://codereview.chromium.org/591693002/diff/1/athena/activity/public/activ...
File athena/activity/public/activity_view_model.h (right):

https://codereview.chromium.org/591693002/diff/1/athena/activity/public/activ...
athena/activity/public/activity_view_model.h:61: virtual scoped_ptr<gfx::Image>
GetOverviewModeImage() = 0;
Why this change?

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy.cc
File athena/content/content_proxy.cc (right):

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:58: };
Any chance this can be consolidated with
NavigationEntryScreenshotManager::ScreenshotData?

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:147:
proxy_content_to_image_factory_.GetWeakPtr()),
InvalidateWeakPtrs() first, so that we ignore an earlier call to getting the
screenshot that is still ongoing. Alternatively, early out from here is there
are live weak-ptrs, which indicates that we are in the process of encoding an
earlier version of the content, and so we don't want to do this again. (I would
go for the first option)

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:154: HideOriginalContent();
Should we hide only if the PNG encoding works? (i.e. in
OnContentImageEncodeComplete() instead)

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:177: image_ = gfx::Image();
Call image_ something else, to clarify that it is the un-encoded image (e.g.
raw_image_).

[oshima, 2014-09-22 18:26:05]

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy.cc
File athena/content/content_proxy.cc (right):

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:128: web_view_->SetFastResize(true);
We may not want to resize at all when hidden? It may become the soruce of junk
when screen rotates.

[Mr4D (OOO till 08-26), 2014-09-23 19:07:34]

Addressed! Please have another look!

https://codereview.chromium.org/591693002/diff/1/athena/activity/public/activ...
File athena/activity/public/activity_view_model.h (right):

https://codereview.chromium.org/591693002/diff/1/athena/activity/public/activ...
athena/activity/public/activity_view_model.h:11: #include "ui/gfx/image/image.h"
Done.

https://codereview.chromium.org/591693002/diff/1/athena/activity/public/activ...
athena/activity/public/activity_view_model.h:61: virtual scoped_ptr<gfx::Image>
GetOverviewModeImage() = 0;
I have found the PNG decoding to be done with gfx::Image. According to
Image::ToImageSkia(): "The returned result is a weak pointer owned by and scoped
to the life of the Image.". So to keep ImageSkia we would have needed to add
some kind of release call. However it appeared to be much simpler if the caller
can use Image::ToImageSkia and release the bits on its own when done.

Makes sense - or do you see a better solution?

https://codereview.chromium.org/591693002/diff/1/athena/activity/public/activ...
athena/activity/public/activity_view_model.h:61: virtual scoped_ptr<gfx::Image>
GetOverviewModeImage() = 0;
Hmm. I was under the misconception that Image was not auto managed, only
SkiaImage. Done.

https://codereview.chromium.org/591693002/diff/1/athena/content/app_activity.cc
File athena/content/app_activity.cc (right):

https://codereview.chromium.org/591693002/diff/1/athena/content/app_activity....
athena/content/app_activity.cc:30: content_proxy_->ContentGetsDestroyed();
No, not yet. This object will be destroyed because of the content being
destroyed, but this call is being called much earlier. As such as long as this
object lives, the content is still there.

https://codereview.chromium.org/591693002/diff/1/athena/content/app_activity.h
File athena/content/app_activity.h (right):

https://codereview.chromium.org/591693002/diff/1/athena/content/app_activity....
athena/content/app_activity.h:31: // Gets the content proxy so that the AppProxy
can take it over.
On 2014/09/22 16:10:28, oshima wrote:
> Does AppProxy mean AppActivityProxy? 

Done.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy.cc
File athena/content/content_proxy.cc (right):

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:24: class ProxyImageData : public
base::RefCountedThreadSafe<ProxyImageData> {
No. The problem is that every activity will have this object and the required
resources to store the uncompressed images will mount.

As such we keep the compressed image here. One step (not yet done though) is to
save the resulting image on disc to re-create upon session restore.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:36: callback.Run();
Adding a DCHECK is possibly a good idea to see a failure early. However - it
does the "correct" thing in case of a failure and the result will be an empty
bitmap.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:58: };
I looked into that at the beginning, but there are several special cases of
handling that I thought it would not be worth while to pursue it (not even
mentioning data structures involved (e.g. NavigationEntryImpl, ScreenshotData)).

If you feel however strongly I will create a more general read-back API (which
is possibly dangerous since it might get called more frequently then we want it
to get called).

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:81: if (image_.IsEmpty())
On 2014/09/22 16:10:28, oshima wrote:
> shouldn't this be !image_.IsEmpty(), or
> maybe checking png_data is better/ (if both are empty, it will return empty
> image)

Done.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:86:
image_reps.push_back(gfx::ImagePNGRep(png_data_, 2.0f));
I wasn't able to test this yet, but according to the comment it states "and the
scale factor which it was intended for". I also looked at the code and the scale
factor was not used for the decode. We are half size and I know that many image
formats have an indication on the size which helps upon reconstruction. I was
under the impression that this is it (without being able to test this yet). But
I have set it to 1.0 and I will revisit when we have something running. In the
end we want to load it into a layer and we transform simple the layer.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:106: if (web_view_ && !content_visible_) {
Yes, it's better to set it.

I didn't want to use the web_view's visibility since someone else might change
that as well and we have changed more things than only the visibility of the
web_view.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:128: web_view_->SetFastResize(true);
An excellent idea. I was thinking of maybe doing what was suggested in line 122.
There is a long reasoning for this which I can explain later. I added more to
the comment in 122.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:147:
proxy_content_to_image_factory_.GetWeakPtr()),
This function gets only called once through the constructor. There will be no
second call (ever - makes simply no sense). However I see your point and added a
DCHECK and variable to make sure that it does not happen in a code refactor.

Note: This object gets only created when an Activity becomes invisible and it
gets destroyed when it becomes visible again.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:154: HideOriginalContent();
I chose this location since I think its the best location because...

If something goes wrong (or did already go wrong with the read back) we end up
with a blank page which will be visible immediately and makes sure that we do
not hold on to resources unnecessarily. (I am afraid that our current way of
turning windows invisible might cause read back problems). Furthermore I think
that the PNG compress can only fail when we are running out of memory - and by
turning the view invisible rather sooner then later we can avoid that situation
;).

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:177: image_ = gfx::Image();
On 2014/09/22 16:13:33, sadrul wrote:
> Call image_ something else, to clarify that it is the un-encoded image (e.g.
> raw_image_).

Done.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy.h
File athena/content/content_proxy.h (right):

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.h:53: scoped_ptr<gfx::Image> GetContentImage();
On 2014/09/22 16:10:28, oshima wrote:
> ditto

Done.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.h:55: // The content is about to get destroyed by
its creator.
As mentioned in another parallel CL: I always considered On.. functions as
observer functions. Apparently this is not true. Done.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.h:72: bool GetContentAsImage();
On 2014/09/22 16:10:28, oshima wrote:
> CreateContentImage ?

Done.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.h:75: void ContentReadBackAsImage(bool success,
const SkBitmap& bitmap);
On 2014/09/22 16:10:28, oshima wrote:
> OnContentImage{Read/Created/..}?

Done.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.h:81: // ContentGetsDestroyed() gets called and
then set to NULL.
On 2014/09/22 16:10:28, oshima wrote:
> can you document ownership?

Done.

[oshima, 2014-09-24 00:40:01]

lgtm with a nit. Let me know if 0.0 scale (= unscaled) doesn't work.

https://codereview.chromium.org/591693002/diff/1/athena/activity/public/activ...
File athena/activity/public/activity_view_model.h (right):

https://codereview.chromium.org/591693002/diff/1/athena/activity/public/activ...
athena/activity/public/activity_view_model.h:61: virtual scoped_ptr<gfx::Image>
GetOverviewModeImage() = 0;
On 2014/09/23 19:07:33, Mr4D wrote:
> I have found the PNG decoding to be done with gfx::Image. According to
> Image::ToImageSkia(): "The returned result is a weak pointer owned by and
scoped
> to the life of the Image.". So to keep ImageSkia we would have needed to add
> some kind of release call. However it appeared to be much simpler if the
caller
> can use Image::ToImageSkia and release the bits on its own when done.
> 
> Makes sense - or do you see a better solution?

gfx::Image stores images in ImageStorage, which is refcounted and shared between
gfx::Image instances originated from the same instance.

I agree that the comment is a bit misleading. It simply means the returned point
is valid so long
as the gfx::Image(s) owning the image are live.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy.cc
File athena/content/content_proxy.cc (right):

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:36: callback.Run();
On 2014/09/23 19:07:33, Mr4D wrote:
> Adding a DCHECK is possibly a good idea to see a failure early. However - it
> does the "correct" thing in case of a failure and the result will be an empty
> bitmap.

Acknowledged.

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:86:
image_reps.push_back(gfx::ImagePNGRep(png_data_, 2.0f));
On 2014/09/23 19:07:33, Mr4D wrote:
> I wasn't able to test this yet, but according to the comment it states "and
the
> scale factor which it was intended for". I also looked at the code and the
scale
> factor was not used for the decode. We are half size and I know that many
image
> formats have an indication on the size which helps upon reconstruction. I was
> under the impression that this is it (without being able to test this yet).
But
> I have set it to 1.0 and I will revisit when we have something running. In the
> end we want to load it into a layer and we transform simple the layer.

The thumbnail should be "unscaled" image (that is, it should simply scaled to
the given pixel size)

The scale should be 0.0f which should indicates that it's unscaled. Let me know
if it didn't work.

https://codereview.chromium.org/591693002/diff/20001/athena/activity/public/a...
File athena/activity/public/activity_view_model.h (right):

https://codereview.chromium.org/591693002/diff/20001/athena/activity/public/a...
athena/activity/public/activity_view_model.h:62: // The ownership gets passed to
the caller.
gfx::Image shares the internal storage, so this may be a bit misleading. How
about something like "it is guaranteed that the images stored in the returned
gfx::Image will not be modified" ?

https://codereview.chromium.org/591693002/diff/20001/athena/activity/public/a...
athena/activity/public/activity_view_model.h:64: virtual gfx::Image
GetOverviewModeImage() = 0;
I think gfx::ImageSkia is enough. You may land it as is, and I'll look into it.

[oshima, 2014-09-24 00:40:28]

On 2014/09/24 00:40:01, oshima wrote:
> lgtm with a nit. Let me know if 0.0 scale (= unscaled) doesn't work.

and address sadrul's comments.

> 
>
https://codereview.chromium.org/591693002/diff/1/athena/activity/public/activ...
> File athena/activity/public/activity_view_model.h (right):
> 
>
https://codereview.chromium.org/591693002/diff/1/athena/activity/public/activ...
> athena/activity/public/activity_view_model.h:61: virtual
scoped_ptr<gfx::Image>
> GetOverviewModeImage() = 0;
> On 2014/09/23 19:07:33, Mr4D wrote:
> > I have found the PNG decoding to be done with gfx::Image. According to
> > Image::ToImageSkia(): "The returned result is a weak pointer owned by and
> scoped
> > to the life of the Image.". So to keep ImageSkia we would have needed to add
> > some kind of release call. However it appeared to be much simpler if the
> caller
> > can use Image::ToImageSkia and release the bits on its own when done.
> > 
> > Makes sense - or do you see a better solution?
> 
> gfx::Image stores images in ImageStorage, which is refcounted and shared
between
> gfx::Image instances originated from the same instance.
> 
> I agree that the comment is a bit misleading. It simply means the returned
point
> is valid so long
> as the gfx::Image(s) owning the image are live.
> 
>
https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy.cc
> File athena/content/content_proxy.cc (right):
> 
>
https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
> athena/content/content_proxy.cc:36: callback.Run();
> On 2014/09/23 19:07:33, Mr4D wrote:
> > Adding a DCHECK is possibly a good idea to see a failure early. However - it
> > does the "correct" thing in case of a failure and the result will be an
empty
> > bitmap.
> 
> Acknowledged.
> 
>
https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
> athena/content/content_proxy.cc:86:
> image_reps.push_back(gfx::ImagePNGRep(png_data_, 2.0f));
> On 2014/09/23 19:07:33, Mr4D wrote:
> > I wasn't able to test this yet, but according to the comment it states "and
> the
> > scale factor which it was intended for". I also looked at the code and the
> scale
> > factor was not used for the decode. We are half size and I know that many
> image
> > formats have an indication on the size which helps upon reconstruction. I
was
> > under the impression that this is it (without being able to test this yet).
> But
> > I have set it to 1.0 and I will revisit when we have something running. In
the
> > end we want to load it into a layer and we transform simple the layer.
> 
> The thumbnail should be "unscaled" image (that is, it should simply scaled to
> the given pixel size)
> 
> The scale should be 0.0f which should indicates that it's unscaled. Let me
know
> if it didn't work.
> 
>
https://codereview.chromium.org/591693002/diff/20001/athena/activity/public/a...
> File athena/activity/public/activity_view_model.h (right):
> 
>
https://codereview.chromium.org/591693002/diff/20001/athena/activity/public/a...
> athena/activity/public/activity_view_model.h:62: // The ownership gets passed
to
> the caller.
> gfx::Image shares the internal storage, so this may be a bit misleading. How
> about something like "it is guaranteed that the images stored in the returned
> gfx::Image will not be modified" ?
> 
>
https://codereview.chromium.org/591693002/diff/20001/athena/activity/public/a...
> athena/activity/public/activity_view_model.h:64: virtual gfx::Image
> GetOverviewModeImage() = 0;
> I think gfx::ImageSkia is enough. You may land it as is, and I'll look into
it.

[Mr4D (OOO till 08-26), 2014-09-24 16:18:25]

Addressed - and also converted gfx::Image to gfx::SkiaImage as requested.
Sadrul, can you have another look?

Note: While testing, I have seen that windows which were never visible will fail
to create images (which isn't really surprising). So we will have to save the
previews as planned and restore with the session restore (which we were planning
to do anyways). But be prepared that you will only see images coming in for
content which was not yet rendered.

A temporary workaround would be to not turn the web_view's content invisible if
it fails and let the content show instead. However - that would also mean that
upon restart and entering the overview mode all windows would start rendering
which is ... slow, jerky and resource intense what we do not want. As such we
will need a little bit longer with this problem.

Note: I did not find a way to convert a PNG directly to ImageSkia, there are
some gfx::internal functions which could do that, but we clearly do not want to
use that. If you have a better idea: Please let me know!

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy.cc
File athena/content/content_proxy.cc (right):

https://codereview.chromium.org/591693002/diff/1/athena/content/content_proxy...
athena/content/content_proxy.cc:86:
image_reps.push_back(gfx::ImagePNGRep(png_data_, 2.0f));
Done.

I tested and the result seems to work. Note: I find a scale factor of 0 rather
odd + I checked unit tests and other places and have seen 1.0 everywhere but not
0.0, but it works ;).

https://codereview.chromium.org/591693002/diff/20001/athena/activity/public/a...
File athena/activity/public/activity_view_model.h (right):

https://codereview.chromium.org/591693002/diff/20001/athena/activity/public/a...
athena/activity/public/activity_view_model.h:62: // The ownership gets passed to
the caller.
Since the ActivityViewModel does not keep a reference, the receiver will
(implicitly) delete the object when the reference disappears. Okay. Will change
comment.

[sadrul, 2014-09-25 16:04:01]

lgtm

https://codereview.chromium.org/591693002/diff/40001/athena/activity/public/a...
File athena/activity/public/activity_view_model.h (right):

https://codereview.chromium.org/591693002/diff/40001/athena/activity/public/a...
athena/activity/public/activity_view_model.h:62: // ActiveViewModel will hold no
reference to the returned image data. As
*ActivityViewModel

[Mr4D (OOO till 08-26), 2014-09-25 16:29:52]

Thanks for your review!

Sadrul, please note that you should only use the web view from GetWebView when
there is no valid image.

https://codereview.chromium.org/591693002/diff/40001/athena/activity/public/a...
File athena/activity/public/activity_view_model.h (right):

https://codereview.chromium.org/591693002/diff/40001/athena/activity/public/a...
athena/activity/public/activity_view_model.h:62: // ActiveViewModel will hold no
reference to the returned image data. As
On 2014/09/25 16:04:01, sadrul wrote:
> *ActivityViewModel

Done.

[Mr4D (OOO till 08-26), 2014-09-25 16:30:01]

The CQ bit was checked by skuhne@chromium.org

[commit-bot: I haz the power, 2014-09-25 16:31:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/591693002/60001

[commit-bot: I haz the power, 2014-09-25 17:12:26]

Committed patchset #4 (id:60001) as 06956bc45dc7902e980ac70873fa50afb76884b3

[commit-bot: I haz the power, 2014-09-25 17:13:32]

Patchset 4 (id:??) landed as
https://crrev.com/7144431a072fad80fc82ee87e7216011c534ef3b
Cr-Commit-Position: refs/heads/master@{#296729}