Oilpan: Completely move core/animations/ to oilpan's heap

Oilpan: Completely move core/animations/ to oilpan's heap

The lifetime relationship between Animation objects are really complicated. The relationship in this CL is illustrated here:
https://docs.google.com/drawings/d/1jOs2hnSRDyVzH3CsEjdD8iq1hF9Kqt5KaZkMW_7Xv-U/edit?usp=sharing

BUG=340522
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=174194

[haraken, 2014-04-11 07:10:36]

The lifetime relationship between Animation, AnimationPlayer, DocumentTimeline,
TimedItem and SampledEffect are very complicated, and I don't think the CL is
handling the lifetime correctly. I need to revise it.

Not yet for review.

[dstockwell, 2014-04-15 05:02:42]

https://codereview.chromium.org/225073004/diff/50001/Source/core/animation/Ac...
File Source/core/animation/ActiveAnimations.h (right):

https://codereview.chromium.org/225073004/diff/50001/Source/core/animation/Ac...
Source/core/animation/ActiveAnimations.h:64: typedef
WillBeHeapHashSet<RawPtrWillBeWeakMember<AnimationPlayer> > AnimationPlayerSet;
Is there a HeapHashCountedSet? Although we don't rely on the count yet we will
in the future.

(animations add players to this set, currently there is at most one animation
per player, but that will change in the future).

[haraken, 2014-04-15 05:03:55]

https://codereview.chromium.org/225073004/diff/50001/Source/core/animation/Ac...
File Source/core/animation/ActiveAnimations.h (right):

https://codereview.chromium.org/225073004/diff/50001/Source/core/animation/Ac...
Source/core/animation/ActiveAnimations.h:64: typedef
WillBeHeapHashSet<RawPtrWillBeWeakMember<AnimationPlayer> > AnimationPlayerSet;
On 2014/04/15 05:02:43, dstockwell wrote:
> Is there a HeapHashCountedSet? Although we don't rely on the count yet we will
> in the future.
> 
> (animations add players to this set, currently there is at most one animation
> per player, but that will change in the future).

Not yet, and that means that we need to implement it. I'll ask erikcorry@.

[haraken, 2014-04-15 05:07:24]

On 2014/04/15 05:03:55, haraken wrote:
>
https://codereview.chromium.org/225073004/diff/50001/Source/core/animation/Ac...
> File Source/core/animation/ActiveAnimations.h (right):
> 
>
https://codereview.chromium.org/225073004/diff/50001/Source/core/animation/Ac...
> Source/core/animation/ActiveAnimations.h:64: typedef
> WillBeHeapHashSet<RawPtrWillBeWeakMember<AnimationPlayer> >
AnimationPlayerSet;
> On 2014/04/15 05:02:43, dstockwell wrote:
> > Is there a HeapHashCountedSet? Although we don't rely on the count yet we
will
> > in the future.
> > 
> > (animations add players to this set, currently there is at most one
animation
> > per player, but that will change in the future).
> 
> Not yet, and that means that we need to implement it. I'll ask erikcorry@.

BTW, as an alternative of HeapHashCountedSet, will
HeapHashMap<Member<AnimationPlayer>, int> work for you? Oilpan already supports
HeapHashMap.

[haraken, 2014-04-29 04:47:34]

PTAL. Now all animations/ tests pass, and it's ready for review.

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/A...
File Source/core/animation/Animation.h (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/A...
Source/core/animation/Animation.h:96: Element* m_target;

Animation and Element should die together, so both Animation::m_target and
ElementRareData::m_activeAnimations should be Members. However, we cannot yet
use a Member to the Node hierarchy, so we have to use a raw pointer for
Animation::m_target at the moment.

This raw pointer is safe because we explicitly clear Animation::m_target in
~Element(). It is also safe to touch the Animation object in ~Element() because
ElementRareData has a persistent handle to the Animation objects (via
ActiveAnimations). Thus it is guaranteed that the Animation objects die in the
next GC cycle of ~Element(), so it is safe to touch Animation::m_target in
~Element().

Once we can use a Member to the Node hierarchy, we can make this logic much
cleaner.

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/S...
File Source/core/animation/SampledEffect.h (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/S...
Source/core/animation/SampledEffect.h:43: RawPtrWillBeMember<Animation>
m_animation;

Conceptually m_player and m_animation are weak members, but they don't need to
be weak members because they are explicitly cleared in SampledEffect::detach().

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/T...
File Source/core/animation/TimedItem.h (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/T...
Source/core/animation/TimedItem.h:144: RawPtrWillBeWeakMember<AnimationPlayer>
m_player;

Actually I don't fully understand this pointer.

- In non-oilpan builds, this is implemented as a weak pointer. However, shanes@
and timothy@ are saying that this should be a strong pointer. Probably the
situation is that (1) in non-oilpan builds we are using a weak pointer and
accepting wrong behavior because it is hard to make it a strong pointer without
oilpan; (2) in oilpan builds we want to make it a strong pointer.

- However, I cannot find an easy way to make it a strong pointer in oilpan
builds as well. The issue is that we need to invoke TimedItem::detach() when the
AnimationPlayer object dies. Since TimedItem::detach() touches on-heap objects,
we cannot invoke it in ~AnimationPlayer(). Thus we need to invoke
TimedItem::detach() in the weak processing of the AnimationPlayer object. This
means that someone needs to retain a weak pointer to the AnimationPlayer, but
I'm not sure who should retain the weak pointer.

- Given the above, at the moment, I'm using a weak pointer in this CL. This
won't fix existing wrong behavior but won't change existing behavior.

Any advice is welcome.

[haraken, 2014-04-29 09:33:20]

dstockwell or shanes or timothy: I'd be happy if you could take a quick look at
the lifetime relationship of the latest CL is correct. Oilpan reviewers are
going to take a close look at details after that.

[Mads Ager (chromium), 2014-04-29 10:29:41]

I'm finding myself having a hard time reasoning about this because I don't
understand the expected life-time of these objects. dstockwell, timloh, will you
guys be a BlinkOn in a couple of weeks. I would like to spent a bit of time with
you guys to draw a picture of the expected life-time of these objects. :)

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/A...
File Source/core/animation/ActiveAnimations.h (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/A...
Source/core/animation/ActiveAnimations.h:67: typedef
WillBeHeapHashMap<RawPtrWillBeMember<AnimationPlayer>, int>
AnimationPlayerCountedSet;
Maybe rename the type to AnimationPlayerSet since it is no longer counted.

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/A...
File Source/core/animation/AnimationPlayerTest.cpp (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/A...
Source/core/animation/AnimationPlayerTest.cpp:724:
Vector<RefPtrWillBeMember<AnimationPlayer> > players;
WillBeHeapVector, otherwise it will not be traced.

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/C...
File Source/core/animation/CompositorPendingAnimations.h (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/C...
Source/core/animation/CompositorPendingAnimations.h:46: class
CompositorPendingAnimations FINAL {
I think this is a part object of Document. Therefore, you should mark it as
DISALLOW_ALLOCATION and change the Persistent heap vectors to normal heap
vectors and trace this from the Document trace method.

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/S...
File Source/core/animation/SampledEffect.h (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/S...
Source/core/animation/SampledEffect.h:43: RawPtrWillBeMember<Animation>
m_animation;
On 2014/04/29 04:47:35, haraken wrote:
> 
> Conceptually m_player and m_animation are weak members, but they don't need to
> be weak members because they are explicitly cleared in
SampledEffect::detach().

There is a mismatch between the comment and the code here and I don't understand
the life-time well enough to figure out what is right here.

If m_player and m_animations are not supposed to keep the player/animation
alive, we should make them weak members and remove the explicit clearing. It is
not clear to me what drives the explicit clearing of these?

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/T...
File Source/core/animation/TimedItem.cpp (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/T...
Source/core/animation/TimedItem.cpp:204: if (!visitor->isAlive(m_player) &&
m_player)
I would turn the condition the other way around and ask for m_player before
asking the visitor if it is alive.

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/T...
File Source/core/animation/TimedItem.h (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/T...
Source/core/animation/TimedItem.h:144: RawPtrWillBeWeakMember<AnimationPlayer>
m_player;
On 2014/04/29 04:47:35, haraken wrote:
> 
> Actually I don't fully understand this pointer.
> 
> - In non-oilpan builds, this is implemented as a weak pointer. However,
shanes@
> and timothy@ are saying that this should be a strong pointer. Probably the
> situation is that (1) in non-oilpan builds we are using a weak pointer and
> accepting wrong behavior because it is hard to make it a strong pointer
without
> oilpan; (2) in oilpan builds we want to make it a strong pointer.
> 
> - However, I cannot find an easy way to make it a strong pointer in oilpan
> builds as well. The issue is that we need to invoke TimedItem::detach() when
the
> AnimationPlayer object dies. Since TimedItem::detach() touches on-heap
objects,
> we cannot invoke it in ~AnimationPlayer(). Thus we need to invoke
> TimedItem::detach() in the weak processing of the AnimationPlayer object. This
> means that someone needs to retain a weak pointer to the AnimationPlayer, but
> I'm not sure who should retain the weak pointer.
> 
> - Given the above, at the moment, I'm using a weak pointer in this CL. This
> won't fix existing wrong behavior but won't change existing behavior.
> 
> Any advice is welcome.

I don't understand the lifetime of these objects well enough. Maybe the authors
of core/animation can create a drawing of the lifetime connections that they
would like to have. Which objects live and die together. Which objects are
allowed to die independently of the other objects?

[dstockwell, 2014-04-29 22:29:57]

On 2014/04/29 10:29:41, Mads Ager (chromium) wrote:
> I'm finding myself having a hard time reasoning about this because I don't
> understand the expected life-time of these objects. dstockwell, timloh, will
you
> guys be a BlinkOn in a couple of weeks. I would like to spent a bit of time
with
> you guys to draw a picture of the expected life-time of these objects. :)

Yes, we'll be at blinkon. But I'll take a look at this patch and try to explain
here too.

[haraken, 2014-04-30 02:48:08]

On 2014/04/29 22:29:57, dstockwell wrote:
> On 2014/04/29 10:29:41, Mads Ager (chromium) wrote:
> > I'm finding myself having a hard time reasoning about this because I don't
> > understand the expected life-time of these objects. dstockwell, timloh, will
> you
> > guys be a BlinkOn in a couple of weeks. I would like to spent a bit of time
> with
> > you guys to draw a picture of the expected life-time of these objects. :)
> 
> Yes, we'll be at blinkon. But I'll take a look at this patch and try to
explain
> here too.

That's helpful since the core/animations is actively being developed and a bit
hard to rebase :) I'd be happy if you could just take a look at the lifetime
relationship of animation objects in this CL. The detailed oilpan syntax will be
reviewed by oilpan guys.

[dstockwell, 2014-05-01 14:06:53]

Sorry for the delay, I left some comments with examples inline. Haven't had a
chance to look through everything yet, but I hope my pointers (ha!) so far will
be helpful.

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/A...
File Source/core/animation/AnimationStack.cpp (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/A...
Source/core/animation/AnimationStack.cpp:142:
m_effects[i]->animation()->notifySampledEffectRemovedFromAnimationStack();
This should probably not be necessary. This cleans up Animation's reference
(m_sampledEffect) to m_effects[i]. Instead m_sampledEffect should be a weak
member.

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/S...
File Source/core/animation/SampledEffect.h (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/S...
Source/core/animation/SampledEffect.h:24: void detach();
'attach/detach' is actually terminology we use in Web Animations to describe the
process of a TimedItem being dis/associated with a Player. That may have been
confusing.

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/S...
Source/core/animation/SampledEffect.h:43: RawPtrWillBeMember<Animation>
m_animation;
On 2014/04/29 04:47:35, haraken wrote:
> 
> Conceptually m_player and m_animation are weak members, but they don't need to
> be weak members because they are explicitly cleared in
SampledEffect::detach().

The explicit clearing was to make up for not having weak members ;)

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/T...
File Source/core/animation/TimedItem.h (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/T...
Source/core/animation/TimedItem.h:144: RawPtrWillBeWeakMember<AnimationPlayer>
m_player;
On 2014/04/29 04:47:35, haraken wrote:
> 
> Actually I don't fully understand this pointer.
> 
> - In non-oilpan builds, this is implemented as a weak pointer. However,
shanes@
> and timothy@ are saying that this should be a strong pointer. Probably the
> situation is that (1) in non-oilpan builds we are using a weak pointer and
> accepting wrong behavior because it is hard to make it a strong pointer
without
> oilpan; (2) in oilpan builds we want to make it a strong pointer.

This is correct. We accept wrong behavior currently as the cases where the
correct lifetimes matter are not shipping yet.
 
> - However, I cannot find an easy way to make it a strong pointer in oilpan
> builds as well. The issue is that we need to invoke TimedItem::detach() when
the
> AnimationPlayer object dies.

I think executing this logic in detach() should not be necessary in oilpan.

Its purpose in the destructor should only be to clean up references to the
now-destructing object. But in oilpan, these objects should die together.

Example: V8 -> TimedItem <-> Player <- DocumentTimeline
- Eventually the animation will complete and DocumentTimeline will drop its
reference to Player.
- At this point, Player is no longer referenced (in broken, non-oilpan TimedItem
only has a weak reference).
- Player is destructed and calls detach on its source TimedItem to clean up
TimedItem's raw pointer.

However, the TimedItem and Player here should strongly reference each other and
die together.

An expanded example (I fail at drawing ascii diagrams, so consider the two
Animations and SampledEffects to be the same objects):
V8 -> Animation <-> Player <- DocumentTimeline
      Animation <-> SampledEffect -> Player
      AnimationStack -> SampledEffect
- Eventually the animation will complete and DocumentTimeline will drop its
reference to Player.
- However, in this example, the animation still has an effect and will continue
to have an effect unless cancelled. In the event that Animation/Player become
unreachable from V8, the effect remains and is no longer cancellable but we need
to simplify these data structures to avoid leaking. The result is that only the
SampledEffect should remain, the Animation and Player should die.
- Currently SampledEffect has a strong reference to Player and a weak reference
to the Animation. When AnimationStack determines that SampledEffect is holding
the last reference to Player and can be simplified, it releases SampledEffect,
allowing Player and Animation to die.
- In Oilpan, SampledEffect's reference to Player should not be needed at all,
instead SampledEffect should only hold a weak reference to Animation.
SampledEffect then only needs to know that Animation has died in order to
complete simplification.

Sorry! It's complicated! I hope this helps :/

@timloh should read this and point out all my mistakes.

[Timothy Loh, 2014-05-02 05:46:00]

As doug said, we won't really need any animations logic in any of the
destructors of AnimationPlayer/TimedItem/Animation, since everything will die
together, and only when the animation is not in effect. For now, we probably
will want to keep notifyAnimationDestroyed, until Element is moved to Oilpan.

I made a diagram of the main classes showing the relationships:
https://docs.google.com/drawings/d/1jOs2hnSRDyVzH3CsEjdD8iq1hF9Kqt5KaZkMW_7Xv...

The main idea is that when the DocumentTimeline drops its reference to an
AnimationPlayer (in serviceAnimations), we should be able to free the
AnimationPlayer and Animation. This is why we have the two weak pointers in the
diagram.

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/A...
File Source/core/animation/ActiveAnimations.h (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/A...
Source/core/animation/ActiveAnimations.h:67: typedef
WillBeHeapHashMap<RawPtrWillBeMember<AnimationPlayer>, int>
AnimationPlayerCountedSet;
This should probably be a weak member.

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/A...
File Source/core/animation/AnimationStack.cpp (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/A...
Source/core/animation/AnimationStack.cpp:142:
m_effects[i]->animation()->notifySampledEffectRemovedFromAnimationStack();
On 2014/05/01 14:06:54, dstockwell wrote:
> This should probably not be necessary. This cleans up Animation's reference
> (m_sampledEffect) to m_effects[i]. Instead m_sampledEffect should be a weak
> member.

We still need this call, since otherwise if you update an animation with a new
effect after this (and before a gc) we wouldn't attach the effect to the stack.

[haraken, 2014-05-02 14:11:48]

Thanks Doug and Timothy!

The diagram is really helpful to understand how the pointers should be
organized. I'm updating a CL accordingly. Let me upload a new version once I fix
all crashes :)

[haraken, 2014-05-04 15:31:46]

Thanks Doug and Timothy, your explanation really helped me understand the
lifetime relationship.

Now all tests in animations/ pass, so PTAL.

> As doug said, we won't really need any animations logic in any of the
> destructors of AnimationPlayer/TimedItem/Animation, since everything will die
> together, and only when the animation is not in effect.

Makes sense. Now I understand that we can simply remove the detach logic from
the destructor of AnimationPlayer. In Oilpan,
AnimationPlayer/TimedItem/Animation die together.

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/A...
File Source/core/animation/ActiveAnimations.h (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/A...
Source/core/animation/ActiveAnimations.h:67: typedef
WillBeHeapHashMap<RawPtrWillBeMember<AnimationPlayer>, int>
AnimationPlayerCountedSet;
On 2014/05/02 05:46:00, Timothy Loh wrote:
> This should probably be a weak member.

Agreed, this should be a weak member.

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/S...
File Source/core/animation/SampledEffect.h (right):

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/S...
Source/core/animation/SampledEffect.h:24: void detach();
On 2014/05/01 14:06:54, dstockwell wrote:
> 'attach/detach' is actually terminology we use in Web Animations to describe
the
> process of a TimedItem being dis/associated with a Player. That may have been
> confusing.

Done. Renamed it back to clear().

https://codereview.chromium.org/225073004/diff/130001/Source/core/animation/S...
Source/core/animation/SampledEffect.h:43: RawPtrWillBeMember<Animation>
m_animation;
On 2014/05/01 14:06:54, dstockwell wrote:
> On 2014/04/29 04:47:35, haraken wrote:
> > 
> > Conceptually m_player and m_animation are weak members, but they don't need
to
> > be weak members because they are explicitly cleared in
> SampledEffect::detach().
> 
> The explicit clearing was to make up for not having weak members ;)

You're right. I made these pointers weak members.

[Mads Ager (chromium), 2014-05-06 17:48:57]

With the Node hierarchy change ongoing this slipped. Haraken, I'll have a look
at this tomorrow morning. Based on Timothy's diagram I actually feel I can
review this now. Thanks Timothy. :)

[haraken, 2014-05-07 00:53:02]

On 2014/05/06 17:48:57, Mads Ager (chromium) wrote:
> With the Node hierarchy change ongoing this slipped. Haraken, I'll have a look
> at this tomorrow morning. Based on Timothy's diagram I actually feel I can
> review this now. Thanks Timothy. :)

Thanks, Mads. I'll rebase after sof@ moves NodeRareData to the heap, and then
let me ping you. It will simplify this CL more.

[haraken, 2014-05-12 08:20:42]

OK, now that the ElementRareData is moved to the heap, I removed the complexity
about the bidirectional reference between Element and Animation (i.e.,
ActiveAnimations::notifyAnimationDestroyed etc).

Now all tests are passing. PTAL.

[Timothy Loh, 2014-05-12 09:47:56]

lgtm

Seems fine to me, but you'll probably want an oilpan reviewer to look over
everything.

[haraken, 2014-05-12 09:48:57]

On 2014/05/12 09:47:56, Timothy Loh wrote:
> lgtm
> 
> Seems fine to me, but you'll probably want an oilpan reviewer to look over
> everything.

Thanks timothy!

[Mads Ager (chromium), 2014-05-15 08:36:53]

LGTM!

https://codereview.chromium.org/225073004/diff/190001/Source/core/animation/D...
File Source/core/animation/DocumentTimeline.h (right):

https://codereview.chromium.org/225073004/diff/190001/Source/core/animation/D...
Source/core/animation/DocumentTimeline.h:60: virtual void trace(Visitor*) = 0;
Let's provide a default implementation for trace to avoid pure virtual trace
methods?

https://codereview.chromium.org/225073004/diff/190001/Source/core/animation/S...
File Source/core/animation/SampledEffect.h (right):

https://codereview.chromium.org/225073004/diff/190001/Source/core/animation/S...
Source/core/animation/SampledEffect.h:40:
RefPtrWillBeWeakMember<AnimationPlayer> m_player;
Instead of making this a WeakMember you can completely remove m_player from the
Oilpan build, right? It looks like it is only used to call the canFree methods
and we don't have to do that with Oilpan.

https://codereview.chromium.org/225073004/diff/190001/Source/core/dom/Documen...
File Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/225073004/diff/190001/Source/core/dom/Documen...
Source/core/dom/Document.cpp:577: #endif
We have multiple blocks of #if !ENABLE(OILPAN) here now. Can you collapse them
all and just remove lines 577, 579, 592 and 596? :)

[haraken, 2014-05-16 07:41:21]

https://codereview.chromium.org/225073004/diff/190001/Source/core/animation/D...
File Source/core/animation/DocumentTimeline.h (right):

https://codereview.chromium.org/225073004/diff/190001/Source/core/animation/D...
Source/core/animation/DocumentTimeline.h:60: virtual void trace(Visitor*) = 0;
On 2014/05/15 08:36:54, Mads Ager (chromium) wrote:
> Let's provide a default implementation for trace to avoid pure virtual trace
> methods?

Done.

https://codereview.chromium.org/225073004/diff/190001/Source/core/animation/S...
File Source/core/animation/SampledEffect.h (right):

https://codereview.chromium.org/225073004/diff/190001/Source/core/animation/S...
Source/core/animation/SampledEffect.h:40:
RefPtrWillBeWeakMember<AnimationPlayer> m_player;
On 2014/05/15 08:36:54, Mads Ager (chromium) wrote:
> Instead of making this a WeakMember you can completely remove m_player from
the
> Oilpan build, right? It looks like it is only used to call the canFree methods
> and we don't have to do that with Oilpan.

Done.

https://codereview.chromium.org/225073004/diff/190001/Source/core/dom/Documen...
File Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/225073004/diff/190001/Source/core/dom/Documen...
Source/core/dom/Document.cpp:577: #endif
On 2014/05/15 08:36:54, Mads Ager (chromium) wrote:
> We have multiple blocks of #if !ENABLE(OILPAN) here now. Can you collapse them
> all and just remove lines 577, 579, 592 and 596? :)

Done.

[haraken, 2014-05-16 07:44:48]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-05-16 07:45:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/haraken@chromium.org/225073004/230001

[commit-bot: I haz the power, 2014-05-16 08:56:57]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/7556)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/7330)
  mac_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/6844)
  win_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_compile_dbg/bu...)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/7322)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[commit-bot: I haz the power, 2014-05-16 09:23:17]

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

[commit-bot: I haz the power, 2014-05-16 09:23:18]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[haraken, 2014-05-16 15:16:00]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-05-16 15:16:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/haraken@chromium.org/225073004/240001

[commit-bot: I haz the power, 2014-05-16 16:42:40]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/7390)

[commit-bot: I haz the power, 2014-05-16 17:47:09]

Change committed as 174194

[Dirk Pranke, 2014-05-16 22:05:58]

This change appears to have caused an inspector layout test to fail.

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=ins...

I suppressed the failure for now and filed a bug:

https://code.google.com/p/chromium/issues/detail?id=374411
http://crrev.com/282423002

[haraken, 2014-05-18 08:30:58]

On 2014/05/16 22:05:58, Dirk Pranke wrote:
> This change appears to have caused an inspector layout test to fail.
> 
>
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=ins...
> 
> I suppressed the failure for now and filed a bug:
> 
> https://code.google.com/p/chromium/issues/detail?id=374411
> http://crrev.com/282423002

It turned out that the failure is not related to this CL. The culprit was this
one: http://src.chromium.org/viewvc/chrome?revision=271052&view=revision