Convert first letter into a pseudo element.

Convert first letter into a pseudo element.

This is a reland of first letter as a psuedo element after the revert
in r184113.

Currently, first letter renderers are created and destroyed through the
updateFirstLetters() call in RenderBlock. This has, historically, been
problematic as we may miss places where the call was required, leading to
accessing invalid memory.

This CL converts the first letter code to use a PseudoElement for the
first-letter instead of creating classes during layout.

With the PseudoElement implementation the two tests which are currently in
LayoutTests/fast/css/first-letter-removed-added.html work as expected.

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

[dsinclair, 2014-10-30 19:45:20]

Patchset #3 (id:40001) has been deleted

[dsinclair, 2014-10-30 19:45:27]

Patchset #2 (id:20001) has been deleted

[dsinclair, 2014-11-11 21:07:36]

dsinclair@chromium.org changed reviewers:
+ jchaffraix@chromium.org

[dsinclair, 2014-11-11 21:07:37]

PTAL.

I've worked through the clusterfuzz issues, and I think this is ready for
re-landing. The linux failures look like they just need rebaselines. Waiting for
the other bots so I can verify the results.

[leviw_travelin_and_unemployed, 2014-11-11 21:22:03]

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

[leviw_travelin_and_unemployed, 2014-11-11 21:22:04]

Dang! What a patch!

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
File Source/core/dom/FirstLetterPseudoElement.cpp (right):

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.cpp:1: /*
You should be able to use the new shorter license text:

// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

[dsinclair, 2014-11-11 21:24:32]

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
File Source/core/dom/FirstLetterPseudoElement.cpp (right):

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.cpp:1: /*
On 2014/11/11 21:22:04, leviw wrote:
> You should be able to use the new shorter license text:
> 
> // Copyright 2014 The Chromium Authors. All rights reserved.
> // Use of this source code is governed by a BSD-style license that can be
> // found in the LICENSE file.

Heh, I played this game last time. I left it as the old license because the vast
majority of this code is moved from RenderBlock. I couldn't figure out if the
new license applied to mostly new-code, or just any newly created file even if
it's mostly old code.

This seemed safer?

[leviw_travelin_and_unemployed, 2014-11-11 21:30:02]

On 2014/11/11 at 21:24:32, dsinclair wrote:
>
https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
> File Source/core/dom/FirstLetterPseudoElement.cpp (right):
> 
>
https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
> Source/core/dom/FirstLetterPseudoElement.cpp:1: /*
> On 2014/11/11 21:22:04, leviw wrote:
> > You should be able to use the new shorter license text:
> > 
> > // Copyright 2014 The Chromium Authors. All rights reserved.
> > // Use of this source code is governed by a BSD-style license that can be
> > // found in the LICENSE file.
> 
> Heh, I played this game last time. I left it as the old license because the
vast majority of this code is moved from RenderBlock. I couldn't figure out if
the new license applied to mostly new-code, or just any newly created file even
if it's mostly old code.
> 
> This seemed safer?

If you have any worries, let's just stick with the old one :)

[Julien - ping for review, 2014-11-12 21:58:51]

It's a big patch so someone else should look at it. I haven't seen anything that
blocking though.

https://codereview.chromium.org/672953002/diff/100001/LayoutTests/fast/css/fi...
File LayoutTests/fast/css/first-letter.html (right):

https://codereview.chromium.org/672953002/diff/100001/LayoutTests/fast/css/fi...
LayoutTests/fast/css/first-letter.html:7: </style>
Let's add a description of what you're testing and what the expected outcome is
(btw there is no expected result for this test, I hope it's ref-test or
check-layout.js one)

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
File Source/core/dom/FirstLetterPseudoElement.cpp (right):

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.cpp:117: // want to consider that
renderer for our text renderer so we go to the sibling.
I don't understand this explanation: why is that correct to consider the
sibling? (I assume they both belong to the pseudo element's render tree so we
don't have lifetime issues)

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.cpp:129: // FIXME: Need to track down
why these conditions are needed.
Do we need a bug?

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.cpp:202: m_detaching = true;
The name of this variable is weird. I thought it would only be set during
detachment but it looks like it just tracks whether we are detached.

We should rename the variable to better match the intent. Also if it just track
if we are detaced, wouldn't renderer() serve the same purpose? (this is likely
completely off but I had to ask :))

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.cpp:306: }
It's a bit sad to have this function duplicated from PseudoElement with some
little tweaks.

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
File Source/core/dom/FirstLetterPseudoElement.h (right):

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.h:62: static bool
isInvalidFirstLetterRenderer(const RenderObject*);
Why is this function exposed instead of having static linkage?

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.h:71: bool m_detaching;
Per Blink style, you need a conjugated verb: m_isDetaching

https://codereview.chromium.org/672953002/diff/100001/Source/core/editing/Tex...
File Source/core/editing/TextIterator.cpp (right):

https://codereview.chromium.org/672953002/diff/100001/Source/core/editing/Tex...
Source/core/editing/TextIterator.cpp:860: if (pseudoElementRenderer &&
pseudoElementRenderer->style()->visibility() == VISIBLE)
Way, way better written:

return pseudoElementRenderer && pseudoElementRenderer->style()->visibility() ==
VISIBLE;

[rune, 2014-11-13 09:12:07]

rune@opera.com changed reviewers:
+ rune@opera.com

[rune, 2014-11-13 09:12:08]

I've looked a bit at the style resolving, and have a question about style
inheritance for the new pseudo element that hangs off of Element. It looks like
we will inherit style for the first-letter element from the Element it's
generated for when attaching, and not from the parent where it's inserted
according to the CSS spec. Looking closer, it seems this is compensated for in
FirstLetterPseudoElement::attachFirstLetterTextRenderers() were a RenderStyle
that inherits from the correct element is set?

Is that handled for style recalc when you just change color on the inner block
(#b in the example below), for instance?

I'm talking about this case:

<style>
#a { color: red; }
#b { color: green; }
#a::first-letter { text-decoration: underline }
</style>
<div id="a">
  <div id="b">First letter should be green and underlined.</div>
</div>

https://codereview.chromium.org/672953002/diff/100001/LayoutTests/fast/css/dy...
File LayoutTests/fast/css/dynamic-class-pseudo-elements.html (right):

https://codereview.chromium.org/672953002/diff/100001/LayoutTests/fast/css/dy...
LayoutTests/fast/css/dynamic-class-pseudo-elements.html:84:
shouldBe("internals.updateStyleAndReturnAffectedElementCount()", "10");
There are two first-letter pseudo elements added here?

[dsinclair, 2014-11-13 17:55:36]

https://codereview.chromium.org/672953002/diff/100001/LayoutTests/fast/css/dy...
File LayoutTests/fast/css/dynamic-class-pseudo-elements.html (right):

https://codereview.chromium.org/672953002/diff/100001/LayoutTests/fast/css/dy...
LayoutTests/fast/css/dynamic-class-pseudo-elements.html:84:
shouldBe("internals.updateStyleAndReturnAffectedElementCount()", "10");
On 2014/11/13 09:12:08, rune wrote:
> There are two first-letter pseudo elements added here?


I verified just to be sure. The updateStyleAndReturnAffectedElementCount()
counts the number of accesses in the StyleResolver. (This is incremented with
didAccess()).

We call didAccess() twice more now, both of them from
pseudoStyleForElementInternal(). The first call is from
createPseudoElementIfNeeded()  and the second call is from
pseudoStyleForElement().

So, I think this is correctly going up by 2.

https://codereview.chromium.org/672953002/diff/100001/LayoutTests/fast/css/fi...
File LayoutTests/fast/css/first-letter.html (right):

https://codereview.chromium.org/672953002/diff/100001/LayoutTests/fast/css/fi...
LayoutTests/fast/css/first-letter.html:7: </style>
On 2014/11/12 21:58:50, Julien Chaffraix - PST wrote:
> Let's add a description of what you're testing and what the expected outcome
is
> (btw there is no expected result for this test, I hope it's ref-test or
> check-layout.js one)

Done.

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
File Source/core/dom/FirstLetterPseudoElement.cpp (right):

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.cpp:117: // want to consider that
renderer for our text renderer so we go to the sibling.
On 2014/11/12 21:58:51, Julien Chaffraix - PST wrote:
> I don't understand this explanation: why is that correct to consider the
> sibling? (I assume they both belong to the pseudo element's render tree so we
> don't have lifetime issues)

The sibling of the first-letter renderer will be the remaining text renderer.
The tree is something like:

RenderBlock
  RenderBlock (anonymous) (first-letter renderer
     RenderTextFragment (has the letter itself)
  RenderTextFragment (the remaining text)

Even if the remaining text renderer is gone that's fine because we'll just exit
the while (firstLetterTextRenderer) check above. We deal with a null renderer
later.

Added a little bit more context to the message.

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.cpp:129: // FIXME: Need to track down
why these conditions are needed.
On 2014/11/12 21:58:51, Julien Chaffraix - PST wrote:
> Do we need a bug?

Removed the comment.

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.cpp:202: m_detaching = true;
On 2014/11/12 21:58:51, Julien Chaffraix - PST wrote:
> The name of this variable is weird. I thought it would only be set during
> detachment but it looks like it just tracks whether we are detached.
> 
> We should rename the variable to better match the intent. Also if it just
track
> if we are detaced, wouldn't renderer() serve the same purpose? (this is likely
> completely off but I had to ask :))


I've added a reset of the variable to false at the end of the method. We need to
track if we're currently detaching because the call
m_remainingTextRenderer->setText() below will trigger a setNeedsUpdate on the
first letter, which causes things to go badly.

This guards that if we get into setNeedsUpdate while detaching we don't attempt
a recalcStyle on the parent node.

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.cpp:306: }
On 2014/11/12 21:58:51, Julien Chaffraix - PST wrote:
> It's a bit sad to have this function duplicated from PseudoElement with some
> little tweaks.

Acknowledged.

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
File Source/core/dom/FirstLetterPseudoElement.h (right):

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.h:62: static bool
isInvalidFirstLetterRenderer(const RenderObject*);
On 2014/11/12 21:58:51, Julien Chaffraix - PST wrote:
> Why is this function exposed instead of having static linkage?

Done. Leftover from when was used elsewhere.

https://codereview.chromium.org/672953002/diff/100001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.h:71: bool m_detaching;
On 2014/11/12 21:58:51, Julien Chaffraix - PST wrote:
> Per Blink style, you need a conjugated verb: m_isDetaching

Done.

https://codereview.chromium.org/672953002/diff/100001/Source/core/editing/Tex...
File Source/core/editing/TextIterator.cpp (right):

https://codereview.chromium.org/672953002/diff/100001/Source/core/editing/Tex...
Source/core/editing/TextIterator.cpp:860: if (pseudoElementRenderer &&
pseudoElementRenderer->style()->visibility() == VISIBLE)
On 2014/11/12 21:58:51, Julien Chaffraix - PST wrote:
> Way, way better written:
> 
> return pseudoElementRenderer && pseudoElementRenderer->style()->visibility()
==
> VISIBLE;

Good catch, thanks.

[dsinclair, 2014-11-13 17:57:37]

On 2014/11/13 09:12:08, rune wrote:
> I've looked a bit at the style resolving, and have a question about style
> inheritance for the new pseudo element that hangs off of Element. It looks
like
> we will inherit style for the first-letter element from the Element it's
> generated for when attaching, and not from the parent where it's inserted
> according to the CSS spec. Looking closer, it seems this is compensated for in
> FirstLetterPseudoElement::attachFirstLetterTextRenderers() were a RenderStyle
> that inherits from the correct element is set?
> 
> Is that handled for style recalc when you just change color on the inner block
> (#b in the example below), for instance?
> 
> I'm talking about this case:
> 
> <style>
> #a { color: red; }
> #b { color: green; }
> #a::first-letter { text-decoration: underline }
> </style>
> <div id="a">
>   <div id="b">First letter should be green and underlined.</div>
> </div>


Thanks a lot for the test case, looks like I get the wrong style.  I've added
this example to fast/css/first-letter.html, I'm losing the underline on the
first-letter. Let me see if I can figure out how to get it working correctly.

[rune, 2014-11-13 18:21:10]

On 2014/11/13 at 17:57:37, dsinclair wrote:
> On 2014/11/13 09:12:08, rune wrote:

> > <style>
> > #a { color: red; }
> > #b { color: green; }
> > #a::first-letter { text-decoration: underline }
> > </style>
> > <div id="a">
> >   <div id="b">First letter should be green and underlined.</div>
> > </div>

> Thanks a lot for the test case, looks like I get the wrong style.  I've added
this example to fast/css/first-letter.html, I'm losing the underline on the
first-letter. Let me see if I can figure out how to get it working correctly.

Sorry, my bad. text-decoration doesn't apply to ::first-letter. My worry was
that you'd get red instead of green on the first letter.

[rune, 2014-11-13 18:25:12]

On 2014/11/13 at 18:21:10, rune wrote:
> On 2014/11/13 at 17:57:37, dsinclair wrote:
> > On 2014/11/13 09:12:08, rune wrote:
> 
> > > <style>
> > > #a { color: red; }
> > > #b { color: green; }
> > > #a::first-letter { text-decoration: underline }
> > > </style>
> > > <div id="a">
> > >   <div id="b">First letter should be green and underlined.</div>
> > > </div>
> 
> > Thanks a lot for the test case, looks like I get the wrong style.  I've
added this example to fast/css/first-letter.html, I'm losing the underline on
the first-letter. Let me see if I can figure out how to get it working
correctly.
> 
> Sorry, my bad. text-decoration doesn't apply to ::first-letter. My worry was
that you'd get red instead of green on the first letter.

No, it does apply according to CSS2.1. No regression though, and it doesn't seem
to work in Gecko either. I need to look more into it.

[dsinclair, 2014-11-13 18:32:26]

On 2014/11/13 18:25:12, rune wrote:
> On 2014/11/13 at 18:21:10, rune wrote:
> > On 2014/11/13 at 17:57:37, dsinclair wrote:
> > > On 2014/11/13 09:12:08, rune wrote:
> > 
> > > > <style>
> > > > #a { color: red; }
> > > > #b { color: green; }
> > > > #a::first-letter { text-decoration: underline }
> > > > </style>
> > > > <div id="a">
> > > >   <div id="b">First letter should be green and underlined.</div>
> > > > </div>
> > 
> > > Thanks a lot for the test case, looks like I get the wrong style.  I've
> added this example to fast/css/first-letter.html, I'm losing the underline on
> the first-letter. Let me see if I can figure out how to get it working
> correctly.
> > 
> > Sorry, my bad. text-decoration doesn't apply to ::first-letter. My worry was
> that you'd get red instead of green on the first letter.
> 
> No, it does apply according to CSS2.1. No regression though, and it doesn't
seem
> to work in Gecko either. I need to look more into it.


Looking at Chrome 40.0.2209.0 (Official Build) dev I see the F as being
underlined in the example you provided. It isn't underlined in FireFox. So, I
think this would be a regression in chrome since, as pointed out,
text-decoration is spec'd as being supported.  (I was hoping it was a
progression instead of a regression, heh.)

[dsinclair, 2014-11-13 21:04:30]

On 2014/11/13 09:12:08, rune wrote:
> I've looked a bit at the style resolving, and have a question about style
> inheritance for the new pseudo element that hangs off of Element. It looks
like
> we will inherit style for the first-letter element from the Element it's
> generated for when attaching, and not from the parent where it's inserted
> according to the CSS spec. Looking closer, it seems this is compensated for in
> FirstLetterPseudoElement::attachFirstLetterTextRenderers() were a RenderStyle
> that inherits from the correct element is set?


Taking a deeper look, and you're right, the code as written is a bit wacky. It's
currently a combination of pseudoElement and the non-pseudoElement code and I
need to clean it up.

Currently it will setup a style in pseudoStyleForElementInternal() which will be
based off of the wrong element. But, we ignore that style completely when we
create the renderer as we call styleForFirstLetter() when we attach the pseudo
element.

This means we're dis-regarding what pseudoStyleForElementInternal() says, but,
it looks like there is code in there around doing things for animation and other
things. If we run that code, get the wrong result, and then make a different
style, does that mean the animation stuff will be wrong?

I think the right solution to this is fixup
StyleResolver::pseudoStyleForElementInternal to do the right thing for
FirstLetter styles and then in attachFirstLetterTextRenderers I don't get a new
style, I just set renderer()->style() onto the first letter renderer.

Then I just need to figure out if didRecalcStyle needs to get a new style or
just pass down the parent renderer()->style() as well.

[dsinclair, 2014-11-14 16:01:07]

Ok, the style regression is on ToT. I filed crbug.com/433334 to track it down. I
think this CL is ready for review again.

I'd like to hold off doing the refactoring of the style
resolver/styleForFirstLetter until after this work lands. I'll file a bug so we
can keep track of it. I don't have enough knowledge of the style system to do
that refactoring safely at the moment.

[Julien - ping for review, 2014-11-14 19:31:00]

lgtm!

http://fc08.deviantart.net/fs70/f/2012/144/9/9/rock_on__by_midzmedia-d50wnk5.jpg

https://codereview.chromium.org/672953002/diff/160001/Source/core/dom/Element...
File Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/672953002/diff/160001/Source/core/dom/Element...
Source/core/dom/Element.cpp:2529: // incorrect results due to setting the first
letter back.
Is there any way we can detect this with some ASSERTs?

https://codereview.chromium.org/672953002/diff/160001/Source/core/dom/FirstLe...
File Source/core/dom/FirstLetterPseudoElement.cpp (right):

https://codereview.chromium.org/672953002/diff/160001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.cpp:202: m_isDetaching = true;
TemporaryChange<bool> isInDetach(&m_isDetaching);

https://codereview.chromium.org/672953002/diff/160001/Source/core/dom/FirstLe...
File Source/core/dom/FirstLetterPseudoElement.h (right):

https://codereview.chromium.org/672953002/diff/160001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.h:70: bool m_isDetaching;
Nit: I would actually call it m_isInDetach (didn't have the full use of the
variable so it was difficult to offer a valuable naming).

[dsinclair, 2014-11-14 20:17:39]

https://codereview.chromium.org/672953002/diff/160001/Source/core/dom/Element...
File Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/672953002/diff/160001/Source/core/dom/Element...
Source/core/dom/Element.cpp:2529: // incorrect results due to setting the first
letter back.
On 2014/11/14 19:31:00, Julien Chaffraix - PST wrote:
> Is there any way we can detect this with some ASSERTs?

I don't think so, the disposed element won't know if it has to set the text on
the remaining text renderer or not. I can't think of any way for it to know
outside of doing something with flag setting?

https://codereview.chromium.org/672953002/diff/160001/Source/core/dom/FirstLe...
File Source/core/dom/FirstLetterPseudoElement.cpp (right):

https://codereview.chromium.org/672953002/diff/160001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.cpp:202: m_isDetaching = true;
On 2014/11/14 19:31:00, Julien Chaffraix - PST wrote:
> TemporaryChange<bool> isInDetach(&m_isDetaching);

Done.

https://codereview.chromium.org/672953002/diff/160001/Source/core/dom/FirstLe...
File Source/core/dom/FirstLetterPseudoElement.h (right):

https://codereview.chromium.org/672953002/diff/160001/Source/core/dom/FirstLe...
Source/core/dom/FirstLetterPseudoElement.h:70: bool m_isDetaching;
On 2014/11/14 19:31:00, Julien Chaffraix - PST wrote:
> Nit: I would actually call it m_isInDetach (didn't have the full use of the
> variable so it was difficult to offer a valuable naming).

Done.

[dsinclair, 2014-11-14 20:17:47]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2014-11-14 20:18:45]

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

[commit-bot: I haz the power, 2014-11-14 20:29:48]

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

[commit-bot: I haz the power, 2014-11-14 20:29:50]

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

[dsinclair, 2014-11-14 20:44:06]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2014-11-14 20:45:37]

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

[commit-bot: I haz the power, 2014-11-14 20:56:56]

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

[commit-bot: I haz the power, 2014-11-14 20:56:58]

Try jobs failed on following builders:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/3...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/3...)

[dsinclair, 2014-11-14 21:12:00]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2014-11-14 21:12:32]

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

[commit-bot: I haz the power, 2014-11-14 23:01:31]

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

[commit-bot: I haz the power, 2014-11-14 23:01:35]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/36612)

[dsinclair, 2014-11-15 02:37:11]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2014-11-15 02:38:03]

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

[commit-bot: I haz the power, 2014-11-15 04:42:13]

Committed patchset #12 (id:260001) as 185405