Enable chromium_code=1 on Windows

Enable chromium_code=1 on Windows

TBR=dglazkov@chromium.org
R=thakis@chromium.org
BUG=312287
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161469

[scottmg, 2013-11-06 04:40:48]

https://codereview.chromium.org/60113005/diff/1/Source/config.gyp
File Source/config.gyp (right):

https://codereview.chromium.org/60113005/diff/1/Source/config.gyp#newcode46
Source/config.gyp:46: 4138, 4244, 4291, 4305, 4344, 4355, 4521, 4099, 4800,
4996,
4800 is "forcing value to bool 'true' or 'false' (performance warning)"

4996 is "using POSIX name instead of ISO name" (in this case strdup instead of
_strdup).

Both seem silly to me. We should probably turn them off in build/common.gypi
too.

https://codereview.chromium.org/60113005/diff/1/Source/core/dom/shadow/Shadow...
File Source/core/dom/shadow/ShadowRoot.cpp (right):

https://codereview.chromium.org/60113005/diff/1/Source/core/dom/shadow/Shadow...
Source/core/dom/shadow/ShadowRoot.cpp:215: if
(static_cast<bool>(m_applyAuthorStyles) == value)
these cause unsigned vs. signed warnings ("unsigned" vs. [signed] "bool"), but
disabling that warning globally doesn't seem a good idea.

https://codereview.chromium.org/60113005/diff/1/Source/platform/text/LocaleWi...
File Source/platform/text/LocaleWin.cpp (right):

https://codereview.chromium.org/60113005/diff/1/Source/platform/text/LocaleWi...
Source/platform/text/LocaleWin.cpp:124: static LCID LCIDFromLocaleInternal(LCID
userDefaultLCID, const String& userDefaultLanguageCode, LocaleNameToLCIDPtr
localeNameToLCID, const String& locale)
conversion from 'WTF::String' to 'WTF::String &'
        A non-const reference may only be bound to an lvalue

seems like just an oversight, as |locale| isn't modified.

[scottmg, 2013-11-06 04:50:17]

There's one outstanding problem in a Release build, warnings of the form:

d:\src\cr\src\third_party\webkit\source\wtf\hashtable.h(332) : error C2220:
warning treated as error - no 'object' file generated
d:\src\cr\src\third_party\webkit\source\wtf\hashtable.h(332) : warning C4714:
function 'WTF::PassRefPtr<T> WTF::AtomicString::add(const char *)' marked as
__forceinline not inlined
        with
        [
            T=WTF::StringImpl
        ]
        d:\src\cr\src\third_party\webkit\source\wtf\text\atomicstring.h(166) :
see declaration of 'WTF::AtomicString::add'
d:\src\cr\src\third_party\webkit\source\wtf\hashtable.h(332) : warning C4714:
function 'WTF::PassRefPtr<T> WTF::AtomicString::add(WTF::StringImpl *)' marked
as __forceinline not inlined
        with
        [
            T=WTF::StringImpl
        ]
        d:\src\cr\src\third_party\webkit\source\wtf\text\atomicstring.h(173) :
see declaration of 'WTF::AtomicString::add'

I'm not sure what to do about that.

I could potentially see why you might want to know about that warning if
__forceinline was use sparingly, but it looks like there's quite a lot of
functions (259) marked ALWAYS_INLINE, so I'm not sure how actionable the warning
really is.

I guess disabling it is no worse than the current state, where the lack of
inlining was of course going undiagnosed?

[scottmg, 2013-11-06 04:57:09]

On 2013/11/06 04:50:17, scottmg wrote:
> There's one outstanding problem in a Release build, warnings of the form:
> 
> d:\src\cr\src\third_party\webkit\source\wtf\hashtable.h(332) : error C2220:
> warning treated as error - no 'object' file generated
> d:\src\cr\src\third_party\webkit\source\wtf\hashtable.h(332) : warning C4714:
> function 'WTF::PassRefPtr<T> WTF::AtomicString::add(const char *)' marked as
> __forceinline not inlined
>         with
>         [
>             T=WTF::StringImpl
>         ]
>         d:\src\cr\src\third_party\webkit\source\wtf\text\atomicstring.h(166) :
> see declaration of 'WTF::AtomicString::add'
> d:\src\cr\src\third_party\webkit\source\wtf\hashtable.h(332) : warning C4714:
> function 'WTF::PassRefPtr<T> WTF::AtomicString::add(WTF::StringImpl *)' marked
> as __forceinline not inlined
>         with
>         [
>             T=WTF::StringImpl
>         ]
>         d:\src\cr\src\third_party\webkit\source\wtf\text\atomicstring.h(173) :
> see declaration of 'WTF::AtomicString::add'
> 
> I'm not sure what to do about that.
> 
> I could potentially see why you might want to know about that warning if
> __forceinline was use sparingly, but it looks like there's quite a lot of
> functions (259) marked ALWAYS_INLINE, so I'm not sure how actionable the
warning
> really is.
> 
> I guess disabling it is no worse than the current state, where the lack of
> inlining was of course going undiagnosed?

(assuming we agree, disabled in ps #3)

[scottmg, 2013-11-06 04:59:08]

Oops, switching reviewer to dglazkov after looking at previous R= on bug.

[Nico, 2013-11-06 05:03:13]

You rock!

Disabling the inlining warning sounds good.

lgtm, but I'd prefer calling accessors of the right type over casting members
(assuming that makes the compiler happy too). Happens in a bunch of places.

https://codereview.chromium.org/60113005/diff/30001/Source/core/dom/shadow/Sh...
File Source/core/dom/shadow/ShadowRoot.cpp (right):

https://codereview.chromium.org/60113005/diff/30001/Source/core/dom/shadow/Sh...
Source/core/dom/shadow/ShadowRoot.cpp:215: if
(static_cast<bool>(m_applyAuthorStyles) == value)
I was about to suggest changin the type of m_applyAuthorStyles instead, but I
guess it's unsigned so that it can be packed into the remainder of
m_numberOfStyles which must stay unsigned.

Can you maybe call applyAuthorStyles() instead? That should return the correct
type.

https://codereview.chromium.org/60113005/diff/30001/Source/core/dom/shadow/Sh...
Source/core/dom/shadow/ShadowRoot.cpp:241: if (value ==
static_cast<bool>(m_resetStyleInheritance))
(same comment)

https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Cli...
File Source/core/rendering/ClipRect.h (right):

https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Cli...
Source/core/rendering/ClipRect.h:141: && static_cast<bool>(m_fixed) ==
other.fixed();
Hm, I'd probably either call functions on both lhs and rhs, or write members on
both lhs and rhs. But up to you.

https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Ren...
File Source/core/rendering/RenderLayer.cpp (right):

https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Ren...
Source/core/rendering/RenderLayer.cpp:3786: bool isSelfPaintingLayer =
shouldBeSelfPaintingLayer();
Ugh, this shadows the member function of the same name. Maybe
newIsSelfPaitingLayer? (then you can call the method instead of casting in the
next line)

https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Ren...
File Source/core/rendering/RenderLayerScrollableArea.cpp (right):

https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Ren...
Source/core/rendering/RenderLayerScrollableArea.cpp:1403: bool
RenderLayerScrollableArea::setNeedsCompositedScrolling(bool
needsCompositedScrolling)
same ugh, same comment about the next line

https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Ren...
File Source/core/rendering/RenderLayerStackingNode.cpp (right):

https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Ren...
Source/core/rendering/RenderLayerStackingNode.cpp:441: if (isNormalFlowOnly ==
static_cast<bool>(m_isNormalFlowOnly))
and again

https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Ren...
Source/core/rendering/RenderLayerStackingNode.cpp:623: if
(static_cast<bool>(m_needsToBeStackingContainer) == needsToBeStackingContainer)
…

[Nico, 2013-11-06 05:04:10]

On 2013/11/06 04:59:08, scottmg wrote:
> Oops, switching reviewer to dglazkov after looking at previous R= on bug.

I think I reviewed most of them, it's just that I don't have approval powers for
Source/config.gyp (but you can TBR=dglazkov for that file. I should maybe make
me a per-file owner of that file.)

[Peter Kasting, 2013-11-06 07:20:01]

https://codereview.chromium.org/60113005/diff/1/Source/config.gyp
File Source/config.gyp (right):

https://codereview.chromium.org/60113005/diff/1/Source/config.gyp#newcode46
Source/config.gyp:46: 4138, 4244, 4291, 4305, 4344, 4355, 4521, 4099, 4800,
4996,
On 2013/11/06 04:40:48, scottmg wrote:
> 4800 is "forcing value to bool 'true' or 'false' (performance warning)"
> 
> 4996 is "using POSIX name instead of ISO name" (in this case strdup instead of
> _strdup).
> 
> Both seem silly to me. We should probably turn them off in build/common.gypi
> too.

I don't think they're huge deals, but what would we lose by simply fixing them? 
The first can be fixed (where existing code is correct) using any of
"static_cast<bool>(...)", "... != 0", or "!!...", and seems like it could
potentially catch cases where people unintentionally use a numeric value as a
boolean one.  For the second, does using thee ISO names create portability
problems?

(Note: We could fix these in a separate CL, if there are a lot.)

https://codereview.chromium.org/60113005/diff/1/Source/config.gyp#newcode49
Source/config.gyp:49: 'chromium_code': 1,
This doesn't go into the variables section at the top?

https://codereview.chromium.org/60113005/diff/30001/Source/core/dom/shadow/Sh...
File Source/core/dom/shadow/ShadowRoot.cpp (right):

https://codereview.chromium.org/60113005/diff/30001/Source/core/dom/shadow/Sh...
Source/core/dom/shadow/ShadowRoot.cpp:215: if
(static_cast<bool>(m_applyAuthorStyles) == value)
On 2013/11/06 05:03:14, Nico (ooo until Nov 12) wrote:
> I was about to suggest changin the type of m_applyAuthorStyles instead, but I
> guess it's unsigned so that it can be packed into the remainder of
> m_numberOfStyles which must stay unsigned.

Based on searches I did online, it looks like the precise behavior here is
implementation-defined, and varies by compilers.  I suppose we could test to see
what the compilers we use do.

Otherwise I'd go with using the accessor functions here, below, and in other
files as well.

https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Cli...
File Source/core/rendering/ClipRect.h (right):

https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Cli...
Source/core/rendering/ClipRect.h:141: && static_cast<bool>(m_fixed) ==
other.fixed();
On 2013/11/06 05:03:14, Nico (ooo until Nov 12) wrote:
> Hm, I'd probably either call functions on both lhs and rhs, or write members
on
> both lhs and rhs. But up to you.

Yeah, calling accessors on both sides seems nice; for consistency I'd also call
accessors for the members above.

[Nico, 2013-11-06 07:33:03]

I believe in practice compilers only combine bitfields of the same type.
On Nov 5, 2013 11:20 PM, <pkasting@chromium.org> wrote:

>
> https://codereview.chromium.org/60113005/diff/1/Source/config.gyp
> File Source/config.gyp (right):
>
> https://codereview.chromium.org/60113005/diff/1/Source/
> config.gyp#newcode46
> Source/config.gyp:46: 4138, 4244, 4291, 4305, 4344, 4355, 4521, 4099,
> 4800, 4996,
> On 2013/11/06 04:40:48, scottmg wrote:
>
>> 4800 is "forcing value to bool 'true' or 'false' (performance
>>
> warning)"
>
>  4996 is "using POSIX name instead of ISO name" (in this case strdup
>>
> instead of
>
>> _strdup).
>>
>
>  Both seem silly to me. We should probably turn them off in
>>
> build/common.gypi
>
>> too.
>>
>
> I don't think they're huge deals, but what would we lose by simply
> fixing them?  The first can be fixed (where existing code is correct)
> using any of "static_cast<bool>(...)", "... != 0", or "!!...", and seems
> like it could potentially catch cases where people unintentionally use a
> numeric value as a boolean one.  For the second, does using thee ISO
> names create portability problems?
>
> (Note: We could fix these in a separate CL, if there are a lot.)
>
> https://codereview.chromium.org/60113005/diff/1/Source/
> config.gyp#newcode49
> Source/config.gyp:49: 'chromium_code': 1,
> This doesn't go into the variables section at the top?
>
> https://codereview.chromium.org/60113005/diff/30001/
> Source/core/dom/shadow/ShadowRoot.cpp
> File Source/core/dom/shadow/ShadowRoot.cpp (right):
>
> https://codereview.chromium.org/60113005/diff/30001/
> Source/core/dom/shadow/ShadowRoot.cpp#newcode215
> Source/core/dom/shadow/ShadowRoot.cpp:215: if
> (static_cast<bool>(m_applyAuthorStyles) == value)
> On 2013/11/06 05:03:14, Nico (ooo until Nov 12) wrote:
>
>> I was about to suggest changin the type of m_applyAuthorStyles
>>
> instead, but I
>
>> guess it's unsigned so that it can be packed into the remainder of
>> m_numberOfStyles which must stay unsigned.
>>
>
> Based on searches I did online, it looks like the precise behavior here
> is implementation-defined, and varies by compilers.  I suppose we could
> test to see what the compilers we use do.
>
> Otherwise I'd go with using the accessor functions here, below, and in
> other files as well.
>
> https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/
> ClipRect.h
> File Source/core/rendering/ClipRect.h (right):
>
> https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/
> ClipRect.h#newcode141
> Source/core/rendering/ClipRect.h:141: && static_cast<bool>(m_fixed) ==
> other.fixed();
> On 2013/11/06 05:03:14, Nico (ooo until Nov 12) wrote:
>
>> Hm, I'd probably either call functions on both lhs and rhs, or write
>>
> members on
>
>> both lhs and rhs. But up to you.
>>
>
> Yeah, calling accessors on both sides seems nice; for consistency I'd
> also call accessors for the members above.
>
> https://codereview.chromium.org/60113005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[scottmg, 2013-11-06 16:11:49]

On 2013/11/06 05:03:13, Nico (ooo until Nov 12) wrote:
> You rock!
> 
> Disabling the inlining warning sounds good.
> 
> lgtm, but I'd prefer calling accessors of the right type over casting members
> (assuming that makes the compiler happy too). Happens in a bunch of places.

Done, with permutations of newXyz.

> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/dom/shadow/Sh...
> File Source/core/dom/shadow/ShadowRoot.cpp (right):
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/dom/shadow/Sh...
> Source/core/dom/shadow/ShadowRoot.cpp:215: if
> (static_cast<bool>(m_applyAuthorStyles) == value)
> I was about to suggest changin the type of m_applyAuthorStyles instead, but I
> guess it's unsigned so that it can be packed into the remainder of
> m_numberOfStyles which must stay unsigned.
> 
> Can you maybe call applyAuthorStyles() instead? That should return the correct
> type.
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/dom/shadow/Sh...
> Source/core/dom/shadow/ShadowRoot.cpp:241: if (value ==
> static_cast<bool>(m_resetStyleInheritance))
> (same comment)
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Cli...
> File Source/core/rendering/ClipRect.h (right):
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Cli...
> Source/core/rendering/ClipRect.h:141: && static_cast<bool>(m_fixed) ==
> other.fixed();
> Hm, I'd probably either call functions on both lhs and rhs, or write members
on
> both lhs and rhs. But up to you.
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Ren...
> File Source/core/rendering/RenderLayer.cpp (right):
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Ren...
> Source/core/rendering/RenderLayer.cpp:3786: bool isSelfPaintingLayer =
> shouldBeSelfPaintingLayer();
> Ugh, this shadows the member function of the same name. Maybe
> newIsSelfPaitingLayer? (then you can call the method instead of casting in the
> next line)
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Ren...
> File Source/core/rendering/RenderLayerScrollableArea.cpp (right):
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Ren...
> Source/core/rendering/RenderLayerScrollableArea.cpp:1403: bool
> RenderLayerScrollableArea::setNeedsCompositedScrolling(bool
> needsCompositedScrolling)
> same ugh, same comment about the next line
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Ren...
> File Source/core/rendering/RenderLayerStackingNode.cpp (right):
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Ren...
> Source/core/rendering/RenderLayerStackingNode.cpp:441: if (isNormalFlowOnly ==
> static_cast<bool>(m_isNormalFlowOnly))
> and again
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Ren...
> Source/core/rendering/RenderLayerStackingNode.cpp:623: if
> (static_cast<bool>(m_needsToBeStackingContainer) ==
needsToBeStackingContainer)
> …

[scottmg, 2013-11-06 16:13:23]

On 2013/11/06 07:20:01, Peter Kasting wrote:
> https://codereview.chromium.org/60113005/diff/1/Source/config.gyp
> File Source/config.gyp (right):
> 
> https://codereview.chromium.org/60113005/diff/1/Source/config.gyp#newcode46
> Source/config.gyp:46: 4138, 4244, 4291, 4305, 4344, 4355, 4521, 4099, 4800,
> 4996,
> On 2013/11/06 04:40:48, scottmg wrote:
> > 4800 is "forcing value to bool 'true' or 'false' (performance warning)"
> > 
> > 4996 is "using POSIX name instead of ISO name" (in this case strdup instead
of
> > _strdup).
> > 
> > Both seem silly to me. We should probably turn them off in build/common.gypi
> > too.
> 
> I don't think they're huge deals, but what would we lose by simply fixing
them? 
> The first can be fixed (where existing code is correct) using any of
> "static_cast<bool>(...)", "... != 0", or "!!...", and seems like it could
> potentially catch cases where people unintentionally use a numeric value as a
> boolean one.  For the second, does using thee ISO names create portability
> problems?
> 
> (Note: We could fix these in a separate CL, if there are a lot.)

I don't think the POSIX one is worth worrying about too much since we're going
to be using that one on other platforms, but I'll see how many there actually
was, and maybe an ifdef is just as easy.

I'll also look into enabling 4800 (bool) in a followup.

> 
> https://codereview.chromium.org/60113005/diff/1/Source/config.gyp#newcode49
> Source/config.gyp:49: 'chromium_code': 1,
> This doesn't go into the variables section at the top?
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/dom/shadow/Sh...
> File Source/core/dom/shadow/ShadowRoot.cpp (right):
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/dom/shadow/Sh...
> Source/core/dom/shadow/ShadowRoot.cpp:215: if
> (static_cast<bool>(m_applyAuthorStyles) == value)
> On 2013/11/06 05:03:14, Nico (ooo until Nov 12) wrote:
> > I was about to suggest changin the type of m_applyAuthorStyles instead, but
I
> > guess it's unsigned so that it can be packed into the remainder of
> > m_numberOfStyles which must stay unsigned.
> 
> Based on searches I did online, it looks like the precise behavior here is
> implementation-defined, and varies by compilers.  I suppose we could test to
see
> what the compilers we use do.
> 
> Otherwise I'd go with using the accessor functions here, below, and in other
> files as well.
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Cli...
> File Source/core/rendering/ClipRect.h (right):
> 
>
https://codereview.chromium.org/60113005/diff/30001/Source/core/rendering/Cli...
> Source/core/rendering/ClipRect.h:141: && static_cast<bool>(m_fixed) ==
> other.fixed();
> On 2013/11/06 05:03:14, Nico (ooo until Nov 12) wrote:
> > Hm, I'd probably either call functions on both lhs and rhs, or write members
> on
> > both lhs and rhs. But up to you.
> 
> Yeah, calling accessors on both sides seems nice; for consistency I'd also
call
> accessors for the members above.

[commit-bot: I haz the power, 2013-11-06 16:14:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scottmg@chromium.org/60113005/300001

[dglazkov, 2013-11-06 17:31:04]

https://codereview.chromium.org/60113005/diff/300001/Source/core/rendering/Re...
File Source/core/rendering/RenderLayerStackingNode.cpp (right):

https://codereview.chromium.org/60113005/diff/300001/Source/core/rendering/Re...
Source/core/rendering/RenderLayerStackingNode.cpp:440: bool newIsNormalFlowOnly
= shouldBeNormalFlowOnly();
Here and elsewhere: would you not be able to resolve shadowing
this->functionName. The "new" prefix just adds confusion.

[scottmg, 2013-11-06 17:45:58]

On 2013/11/06 17:31:04, Dimitri Glazkov wrote:
>
https://codereview.chromium.org/60113005/diff/300001/Source/core/rendering/Re...
> File Source/core/rendering/RenderLayerStackingNode.cpp (right):
> 
>
https://codereview.chromium.org/60113005/diff/300001/Source/core/rendering/Re...
> Source/core/rendering/RenderLayerStackingNode.cpp:440: bool
newIsNormalFlowOnly
> = shouldBeNormalFlowOnly();
> Here and elsewhere: would you not be able to resolve shadowing
> this->functionName. The "new" prefix just adds confusion.

Ah yes, I like that better. Done.

[commit-bot: I haz the power, 2013-11-06 17:46:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scottmg@chromium.org/60113005/430001

[Peter Kasting, 2013-11-06 18:41:45]

https://codereview.chromium.org/60113005/diff/300001/Source/core/rendering/Re...
File Source/core/rendering/RenderLayerStackingNode.cpp (right):

https://codereview.chromium.org/60113005/diff/300001/Source/core/rendering/Re...
Source/core/rendering/RenderLayerStackingNode.cpp:440: bool newIsNormalFlowOnly
= shouldBeNormalFlowOnly();
On 2013/11/06 17:31:05, Dimitri Glazkov wrote:
> Here and elsewhere: would you not be able to resolve shadowing
> this->functionName. The "new" prefix just adds confusion.

Personally I think the "new" route reads more clearly, but it sounds like you
guys don't agree :)

[Nico, 2013-11-06 19:07:08]

On Wed, Nov 6, 2013 at 10:41 AM, <pkasting@chromium.org> wrote:

>
> https://codereview.chromium.org/60113005/diff/300001/
> Source/core/rendering/RenderLayerStackingNode.cpp
> File Source/core/rendering/RenderLayerStackingNode.cpp (right):
>
> https://codereview.chromium.org/60113005/diff/300001/
> Source/core/rendering/RenderLayerStackingNode.cpp#newcode440
> Source/core/rendering/RenderLayerStackingNode.cpp:440: bool
> newIsNormalFlowOnly = shouldBeNormalFlowOnly();
> On 2013/11/06 17:31:05, Dimitri Glazkov wrote:
>
>> Here and elsewhere: would you not be able to resolve shadowing
>> this->functionName. The "new" prefix just adds confusion.
>>
>
> Personally I think the "new" route reads more clearly, but it sounds
> like you guys don't agree :)
>

I agree with pkasting, but it can land either way I think.


>
> https://codereview.chromium.org/60113005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2013-11-06 20:02:19]

Change committed as 161469