Introduce Property Trees

Introduce Property Trees

Based of Andrew's work: crrev.com/642833002

This patch creates a transform and a clip tree and computes
visual content rects based on those.

BUG=386786
Committed: https://crrev.com/51ed1a26449d0beba197c96172436d493b27bfb5
Cr-Commit-Position: refs/heads/master@{#308724}

[awoloszyn, 2014-11-12 19:38:58]

awoloszyn@chromium.org changed reviewers:
+ awoloszyn@chromium.org

[awoloszyn, 2014-11-12 19:38:59]

https://codereview.chromium.org/687873004/diff/40001/cc/trees/property_tree_b...
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/687873004/diff/40001/cc/trees/property_tree_b...
cc/trees/property_tree_builder.cc:212: layer->position().OffsetFromOrigin() +
That is way nicer.

https://codereview.chromium.org/687873004/diff/40001/cc/trees/property_tree_b...
cc/trees/property_tree_builder.cc:239: GetTransformParent(data_from_ancestor,
layer);
GetTransformParent does not take into account scroll_parents. The old logic was
if (layer->scroll_parent()) { layer->set_tran.....(layer->scroll_parent()) }

https://codereview.chromium.org/687873004/diff/40001/cc/trees/property_tree_b...
cc/trees/property_tree_builder.cc:245: if (layer->clip_parent()) {
This line is redundant.

[Ian Vollick, 2014-12-10 22:10:15]

vollick@chromium.org changed reviewers:
+ enne@chromium.org

[enne (OOO), 2014-12-10 23:46:00]

This is looking really good.  From our discussions offline, it sounded like you
also wanted to add a command-line / LayerTreeSetting to do the DCHECKing for
visible rects, and we could then just run that for all layout tests and cc unit
tests.  That to me seems like it would make this landable.

What are the next steps after that?  awoloszyn's render target work?  Verifying
draw/screen-space transforms as well?  Perf tests?

https://codereview.chromium.org/687873004/diff/560001/cc/layers/layer.h
File cc/layers/layer.h (right):

https://codereview.chromium.org/687873004/diff/560001/cc/layers/layer.h#newco...
cc/layers/layer.h:249: bool has_render_target() const {
What does this mean? When does something not have a render target?

https://codereview.chromium.org/687873004/diff/560001/cc/layers/layer.h#newco...
cc/layers/layer.h:490: const gfx::Rect& debug_visible_rect() const { return
debug_visible_rect_; }
bikeshedding, but maybe visible_rect_from_property_tree? Debug isn't really an
amazing name.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/687873004/diff/560001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:26: // TODO(ajuma): Compute content_scale rather
than using it. Note that for
I do wonder if we could punt on this until impl-side painting is everywhere. 
It's pretty close!

https://codereview.chromium.org/687873004/diff/560001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:26: // TODO(ajuma): Compute content_scale rather
than using it. Note that for
I do wonder if we could punt on this until impl-side painting is everywhere. 
It's pretty close!

https://codereview.chromium.org/687873004/diff/560001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:57: if (!success)
What happens if this doesn't succeed? When does this happen?

https://codereview.chromium.org/687873004/diff/560001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:142: if (!success)
What happens if this doesn't succeed?

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree....
cc/trees/property_tree.cc:125: bool ComputeTransform(const TransformTree& tree,
This probably needs a note that transform should be initialized to identity? Or,
maybe it'd be safer to initialize to identity in that first case and not
initialize the transform that you pass in here?

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree.h
File cc/trees/property_tree.h (right):

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree....
cc/trees/property_tree.h:64: // Not virtual. Should not derive from this class.
final?

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree....
cc/trees/property_tree.h:95: bool CC_EXPORT ComputeTransform(const
TransformTree& tree,
Make this a const function on TransformTree?

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree....
cc/trees/property_tree.h:101: Are2DAxisAligned(const TransformTree& tree, int
source_id, int dest_id);
Same here?

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:71: // TODO(vollick): once Andrew refactors
the surface determinations out of
What's the status on this work?

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:140: // TODO(vollick): This is to mimic bugs
in current CDP. Need to log a bug
!!!

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:272: // We only need to clip the root if we
have a non-empty viewport. The root of
When is the viewport empty?

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:285: device_transform *
transform_root->data.to_parent;
Is this ever not identity? Doesn't the transform root not have a parent?

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:302: clip_root->data.clip =
transformed_viewport;
Why do you need to inverse transform the viewport to get the clip rect?

[Ian Vollick, 2014-12-12 03:01:48]

I'm still trying to figure out how to enable the checks for the unit tests, but
in the meantime, I thought I'd post my responses to your other review comments
(and the related patch).

https://codereview.chromium.org/687873004/diff/560001/cc/layers/layer.h
File cc/layers/layer.h (right):

https://codereview.chromium.org/687873004/diff/560001/cc/layers/layer.h#newco...
cc/layers/layer.h:249: bool has_render_target() const {
On 2014/12/10 23:45:59, enne wrote:
> What does this mean? When does something not have a render target?

Yeah, this surprised me too. It can happen when, eg, an ancestor has a singular
transform and the subtree is skipped. Checking !!render_target() is a no-go in
this case because the DCHECK gets tripped.

https://codereview.chromium.org/687873004/diff/560001/cc/layers/layer.h#newco...
cc/layers/layer.h:490: const gfx::Rect& debug_visible_rect() const { return
debug_visible_rect_; }
On 2014/12/10 23:45:59, enne wrote:
> bikeshedding, but maybe visible_rect_from_property_tree? Debug isn't really an
> amazing name.

Done.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/687873004/diff/560001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:26: // TODO(ajuma): Compute content_scale rather
than using it. Note that for
On 2014/12/10 23:45:59, enne wrote:
> I do wonder if we could punt on this until impl-side painting is everywhere. 
> It's pretty close!

sgtm! I've added another comment saying that once impl-painting is on everywhere
then all this can be ditched.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:57: if (!success)
On 2014/12/10 23:45:59, enne wrote:
> What happens if this doesn't succeed? When does this happen?

If this doesn't succeed it means that an ancestor has a singular transform. In
that case, the element shouldn't draw, so the visible rects don't matter. I've
added some comments about this.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:142: if (!success)
On 2014/12/10 23:45:59, enne wrote:
> What happens if this doesn't succeed?

Added a comment with an explanation.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree....
cc/trees/property_tree.cc:125: bool ComputeTransform(const TransformTree& tree,
On 2014/12/10 23:45:59, enne wrote:
> This probably needs a note that transform should be initialized to identity?
Or,
> maybe it'd be safer to initialize to identity in that first case and not
> initialize the transform that you pass in here?

I actually take advantage of the fact that it doesn't reset to identity to tack
on some scales and transforms before making this call. Check out lth.cc and
Layer::draw_transform_from_property_trees. wdyt, too ugly?

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree.h
File cc/trees/property_tree.h (right):

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree....
cc/trees/property_tree.h:64: // Not virtual. Should not derive from this class.
On 2014/12/10 23:45:59, enne wrote:
> final?

Done.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree....
cc/trees/property_tree.h:95: bool CC_EXPORT ComputeTransform(const
TransformTree& tree,
On 2014/12/10 23:45:59, enne wrote:
> Make this a const function on TransformTree?

Done.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree....
cc/trees/property_tree.h:101: Are2DAxisAligned(const TransformTree& tree, int
source_id, int dest_id);
On 2014/12/10 23:45:59, enne wrote:
> Same here?

Done.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:71: // TODO(vollick): once Andrew refactors
the surface determinations out of
On 2014/12/10 23:45:59, enne wrote:
> What's the status on this work?

Andrew's got a patch up for review. Ball's in Dana's court at the moment.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:140: // TODO(vollick): This is to mimic bugs
in current CDP. Need to log a bug
On 2014/12/10 23:45:59, enne wrote:
> !!!

Yeah :( I've logged the bug and referenced it in this comment.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:272: // We only need to clip the root if we
have a non-empty viewport. The root of
On 2014/12/10 23:46:00, enne wrote:
> When is the viewport empty?

This happens in unit tests and the assumption there is that we don't clip
anything.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:285: device_transform *
transform_root->data.to_parent;
On 2014/12/10 23:45:59, enne wrote:
> Is this ever not identity? Doesn't the transform root not have a parent?

The dsf (and device transform) are applied to the root transform. The to_parent
here can include the dsf.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:302: clip_root->data.clip =
transformed_viewport;
On 2014/12/10 23:46:00, enne wrote:
> Why do you need to inverse transform the viewport to get the clip rect?

To get to screen space (where this clip is applied), you use
ComputeTransform(src, -1). That is, it does include the transform of node 0.
This projection ensures that the clip is correct in screen space.

[enne (OOO), 2014-12-12 19:05:49]

https://codereview.chromium.org/687873004/diff/560001/cc/layers/layer.h
File cc/layers/layer.h (right):

https://codereview.chromium.org/687873004/diff/560001/cc/layers/layer.h#newco...
cc/layers/layer.h:249: bool has_render_target() const {
On 2014/12/12 03:01:47, vollick wrote:
> On 2014/12/10 23:45:59, enne wrote:
> > What does this mean? When does something not have a render target?
> 
> Yeah, this surprised me too. It can happen when, eg, an ancestor has a
singular
> transform and the subtree is skipped. Checking !!render_target() is a no-go in
> this case because the DCHECK gets tripped.

If the subtree is skipped, why are you asking for its draw transform? etc?

https://codereview.chromium.org/687873004/diff/560001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/687873004/diff/560001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:57: if (!success)
On 2014/12/12 03:01:47, vollick wrote:
> On 2014/12/10 23:45:59, enne wrote:
> > What happens if this doesn't succeed? When does this happen?
> 
> If this doesn't succeed it means that an ancestor has a singular transform. In
> that case, the element shouldn't draw, so the visible rects don't matter. I've
> added some comments about this.

Could this just be a DCHECK instead?

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree....
cc/trees/property_tree.cc:125: bool ComputeTransform(const TransformTree& tree,
On 2014/12/12 03:01:47, vollick wrote:
> On 2014/12/10 23:45:59, enne wrote:
> > This probably needs a note that transform should be initialized to identity?
> Or,
> > maybe it'd be safer to initialize to identity in that first case and not
> > initialize the transform that you pass in here?
> 
> I actually take advantage of the fact that it doesn't reset to identity to
tack
> on some scales and transforms before making this call. Check out lth.cc and
> Layer::draw_transform_from_property_trees. wdyt, too ugly?

It still looks to me like you're just passing identity ssxform into
ComputeTransform in that function.  Am I missing something?

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:272: // We only need to clip the root if we
have a non-empty viewport. The root of
On 2014/12/12 03:01:47, vollick wrote:
> On 2014/12/10 23:46:00, enne wrote:
> > When is the viewport empty?
> 
> This happens in unit tests and the assumption there is that we don't clip
> anything.

Can you leave a comment saying that empty viewport = no clipping, and only for
unit tests?  (I suppose we could also fix that too?)

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:302: clip_root->data.clip =
transformed_viewport;
On 2014/12/12 03:01:47, vollick wrote:
> On 2014/12/10 23:46:00, enne wrote:
> > Why do you need to inverse transform the viewport to get the clip rect?
> 
> To get to screen space (where this clip is applied), you use
> ComputeTransform(src, -1). That is, it does include the transform of node 0.
> This projection ensures that the clip is correct in screen space.

Hmm.  Maybe I just figured that this function would just take the device
viewport and set it directly at the top.

[Ian Vollick, 2014-12-15 21:45:17]

PTAL!

https://codereview.chromium.org/687873004/diff/560001/cc/layers/layer.h
File cc/layers/layer.h (right):

https://codereview.chromium.org/687873004/diff/560001/cc/layers/layer.h#newco...
cc/layers/layer.h:249: bool has_render_target() const {
On 2014/12/12 19:05:48, enne wrote:
> On 2014/12/12 03:01:47, vollick wrote:
> > On 2014/12/10 23:45:59, enne wrote:
> > > What does this mean? When does something not have a render target?
> > 
> > Yeah, this surprised me too. It can happen when, eg, an ancestor has a
> singular
> > transform and the subtree is skipped. Checking !!render_target() is a no-go
in
> > this case because the DCHECK gets tripped.
> 
> If the subtree is skipped, why are you asking for its draw transform? etc?

Turns out that there are quite a few reasons we skip layers. Relying on the
presence of the render target turned out to be just a shortcut, preventing me
from writing this logic. I've updated draw_property_utils with this logic (see
FindLayersThatNeedVisibleRects).

https://codereview.chromium.org/687873004/diff/560001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/687873004/diff/560001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:57: if (!success)
On 2014/12/12 19:05:49, enne wrote:
> On 2014/12/12 03:01:47, vollick wrote:
> > On 2014/12/10 23:45:59, enne wrote:
> > > What happens if this doesn't succeed? When does this happen?
> > 
> > If this doesn't succeed it means that an ancestor has a singular transform.
In
> > that case, the element shouldn't draw, so the visible rects don't matter.
I've
> > added some comments about this.
> 
> Could this just be a DCHECK instead?

Done.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree....
cc/trees/property_tree.cc:125: bool ComputeTransform(const TransformTree& tree,
On 2014/12/12 19:05:49, enne wrote:
> On 2014/12/12 03:01:47, vollick wrote:
> > On 2014/12/10 23:45:59, enne wrote:
> > > This probably needs a note that transform should be initialized to
identity?
> > Or,
> > > maybe it'd be safer to initialize to identity in that first case and not
> > > initialize the transform that you pass in here?
> > 
> > I actually take advantage of the fact that it doesn't reset to identity to
> tack
> > on some scales and transforms before making this call. Check out lth.cc and
> > Layer::draw_transform_from_property_trees. wdyt, too ugly?
> 
> It still looks to me like you're just passing identity ssxform into
> ComputeTransform in that function.  Am I missing something?

Sorry, I pointed you at the wrong function. But in any case, I've taken your
suggestion and done a MakeIdentity.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:272: // We only need to clip the root if we
have a non-empty viewport. The root of
On 2014/12/12 19:05:49, enne wrote:
> On 2014/12/12 03:01:47, vollick wrote:
> > On 2014/12/10 23:46:00, enne wrote:
> > > When is the viewport empty?
> > 
> > This happens in unit tests and the assumption there is that we don't clip
> > anything.
> 
> Can you leave a comment saying that empty viewport = no clipping, and only for
> unit tests?  (I suppose we could also fix that too?)

This is now unnecessary due to hush's patch. Removed.

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:285: device_transform *
transform_root->data.to_parent;
On 2014/12/12 03:01:47, vollick wrote:
> On 2014/12/10 23:45:59, enne wrote:
> > Is this ever not identity? Doesn't the transform root not have a parent?
> 
> The dsf (and device transform) are applied to the root transform. The
to_parent
> here can include the dsf.

I've added an extra "super root" to the transform tree that's always the
identity. This gets rid of this confusion, I think?

https://codereview.chromium.org/687873004/diff/560001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:302: clip_root->data.clip =
transformed_viewport;
On 2014/12/12 19:05:49, enne wrote:
> On 2014/12/12 03:01:47, vollick wrote:
> > On 2014/12/10 23:46:00, enne wrote:
> > > Why do you need to inverse transform the viewport to get the clip rect?
> > 
> > To get to screen space (where this clip is applied), you use
> > ComputeTransform(src, -1). That is, it does include the transform of node 0.
> > This projection ensures that the clip is correct in screen space.
> 
> Hmm.  Maybe I just figured that this function would just take the device
> viewport and set it directly at the top.

Now that we have the super root, we don't need to transform the clip. It's
stored in that space.

[enne (OOO), 2014-12-16 19:54:28]

https://codereview.chromium.org/687873004/diff/640001/cc/layers/layer.h
File cc/layers/layer.h (right):

https://codereview.chromium.org/687873004/diff/640001/cc/layers/layer.h#newco...
cc/layers/layer.h:461: bool NeedsVisibleRectUpdated() const;
Could you just call DrawsContent directly elsewhere and not add another
function? This name was confusing to me; I thought it was a stateful dirty flag,
i.e. "this layer's visible rect is dirty and hasn't been updated yet".

https://codereview.chromium.org/687873004/diff/640001/cc/layers/layer.h#newco...
cc/layers/layer.h:660: bool should_render_to_separate_surface_ : 1;
You could call this has_render_surface_ to merge with awoloszyn's patch (and
reorder it)?

https://codereview.chromium.org/687873004/diff/640001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_unittest_no_message_loop.cc (right):

https://codereview.chromium.org/687873004/diff/640001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_no_message_loop.cc:104:
settings.verify_property_trees = true;
Defaulting this property to false does mean that we're not going to end up with
accidentally verifying in production, but is this really all of them?

[Ian Vollick, 2014-12-16 21:28:19]

Thanks for the review!

https://codereview.chromium.org/687873004/diff/640001/cc/layers/layer.h
File cc/layers/layer.h (right):

https://codereview.chromium.org/687873004/diff/640001/cc/layers/layer.h#newco...
cc/layers/layer.h:461: bool NeedsVisibleRectUpdated() const;
On 2014/12/16 19:54:28, enne wrote:
> Could you just call DrawsContent directly elsewhere and not add another
> function? This name was confusing to me; I thought it was a stateful dirty
flag,
> i.e. "this layer's visible rect is dirty and hasn't been updated yet".

Done.

https://codereview.chromium.org/687873004/diff/640001/cc/layers/layer.h#newco...
cc/layers/layer.h:660: bool should_render_to_separate_surface_ : 1;
On 2014/12/16 19:54:28, enne wrote:
> You could call this has_render_surface_ to merge with awoloszyn's patch (and
> reorder it)?

Done.

https://codereview.chromium.org/687873004/diff/640001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_unittest_no_message_loop.cc (right):

https://codereview.chromium.org/687873004/diff/640001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_no_message_loop.cc:104:
settings.verify_property_trees = true;
On 2014/12/16 19:54:28, enne wrote:
> Defaulting this property to false does mean that we're not going to end up
with
> accidentally verifying in production, but is this really all of them?

There are a number of changes in this CL that ensure that we get coverage
(another place to look is the CalcDrawPropsInputForTesting where I set this
value to true). I ensured that the coverage was sufficient by CHECK'ing that
we'd set the verify_property_trees value to true and making sure the tests all
passed.

I thought that the default-off/turn-on-for-tests was a safer design than
default-on/disable-for-production given all the ways we initialize in our
various configurations.

[enne (OOO), 2014-12-16 21:45:16]

lgtm!

[Ian Vollick, 2014-12-16 21:45:56]

The CQ bit was checked by vollick@chromium.org

[commit-bot: I haz the power, 2014-12-16 21:46:28]

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

[commit-bot: I haz the power, 2014-12-16 21:56:57]

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

[commit-bot: I haz the power, 2014-12-16 21:56:58]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[Ian Vollick, 2014-12-17 00:20:10]

The CQ bit was checked by vollick@chromium.org

[commit-bot: I haz the power, 2014-12-17 00:21:20]

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

[commit-bot: I haz the power, 2014-12-17 02:03:12]

Committed patchset #35 (id:680001)

[commit-bot: I haz the power, 2014-12-17 02:04:06]

Patchset 35 (id:??) landed as
https://crrev.com/51ed1a26449d0beba197c96172436d493b27bfb5
Cr-Commit-Position: refs/heads/master@{#308724}