Change MouseEvent.button to be 'short' instead of 'unsigned short'

Change MouseEvent.button to be 'short' instead of 'unsigned short'

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

[ramya.v, 2015-06-09 06:16:25]

ramya.v@samsung.com changed reviewers:
+ philipj@opera.com, rbyers@chromium.org

[ramya.v, 2015-06-09 06:16:26]

PTAL!

Thanks

[philipj_slow, 2015-06-09 09:35:39]

Looks promising, hope we can get rid of the special-casing of (unsigned
short)-1. Rick, what do you think?

https://codereview.chromium.org/1165693005/diff/40001/Source/core/events/Mous...
File Source/core/events/MouseEvent.cpp (right):

https://codereview.chromium.org/1165693005/diff/40001/Source/core/events/Mous...
Source/core/events/MouseEvent.cpp:94: , m_button(button == -1 ? 0 : button)
Getting rid of these casts will be nice. However, I think that coercing -1 to 0
is wrong for the web-exposed ways of creating a MouseEvent, and Firefox doesn't
do it in this test:
http://jsconsole.com/?new%20MouseEvent%28%27click%27%2C{button%3A-1}%29.button

I tested IE, it doesn't have the MouseEvent contructor, but this also allows -1
to pass through:
var event = document.createEvent("MouseEvent");
event.initMouseEvent('click',false,false,null,0,0,0,0,0,false,false,false,false,-1,null);
alert(event.button);

However, strangely enough, passing -2 to IE results in button 65534, just like
Blink/WebKit.

I think the right thing to do here is to simply pass through the values as they
are, as the spec doesn't require any coercion AFAICT. You will need to check if
there are any internal uses reaching any of these code paths where (unsigned
short)-1 will be passed, by any name.
https://code.google.com/p/chromium/codesearch is a good place to dig into that
kind of question.

https://codereview.chromium.org/1165693005/diff/40001/Source/core/events/Mous...
File Source/core/events/MouseEvent.idl (right):

https://codereview.chromium.org/1165693005/diff/40001/Source/core/events/Mous...
Source/core/events/MouseEvent.idl:34: readonly attribute short   button;
Please add some whitespace so that all the names line up to the right.

[ramya.v, 2015-06-09 11:14:31]

https://codereview.chromium.org/1165693005/diff/40001/Source/core/events/Mous...
File Source/core/events/MouseEvent.cpp (right):

https://codereview.chromium.org/1165693005/diff/40001/Source/core/events/Mous...
Source/core/events/MouseEvent.cpp:94: , m_button(button == -1 ? 0 : button)
On 2015/06/09 09:35:38, philipj wrote:
> Getting rid of these casts will be nice. However, I think that coercing -1 to
0
> is wrong for the web-exposed ways of creating a MouseEvent, and Firefox
doesn't
> do it in this test:
> http://jsconsole.com/?new%20MouseEvent%28%27click%27%2C{button%3A-1}%29.button
> 
> I tested IE, it doesn't have the MouseEvent contructor, but this also allows
-1
> to pass through:
> var event = document.createEvent("MouseEvent");
>
event.initMouseEvent('click',false,false,null,0,0,0,0,0,false,false,false,false,-1,null);
> alert(event.button);
> 
> However, strangely enough, passing -2 to IE results in button 65534, just like
> Blink/WebKit.
> 
> I think the right thing to do here is to simply pass through the values as
they
> are, as the spec doesn't require any coercion AFAICT. You will need to check
if
> there are any internal uses reaching any of these code paths where (unsigned
> short)-1 will be passed, by any name.
> https://code.google.com/p/chromium/codesearch is a good place to dig into that
> kind of question.

I've checked for any internal paths which could pass value of -1. Apart form
fakeMouseMoves from EventHandler.cpp, PopupMenuImpl::setValueAndClosePopup is
creating default PlatformMouseEvent constructor in which default value of button
is -1.
Apart from these I could not find any specific place where they are sending
explicit value of -1.

The check (unsigned short)-1?0:button might be added because negative values are
not allowed to be assigned to button since it is unsigned short. Now as you
suggested probably we can pass the values as they are similar to firefox
behavior.

https://codereview.chromium.org/1165693005/diff/40001/Source/core/events/Mous...
File Source/core/events/MouseEvent.idl (right):

https://codereview.chromium.org/1165693005/diff/40001/Source/core/events/Mous...
Source/core/events/MouseEvent.idl:34: readonly attribute short   button;
On 2015/06/09 09:35:38, philipj wrote:
> Please add some whitespace so that all the names line up to the right.

Done.

[philipj_slow, 2015-06-09 12:04:07]

https://codereview.chromium.org/1165693005/diff/40001/Source/core/events/Mous...
File Source/core/events/MouseEvent.cpp (right):

https://codereview.chromium.org/1165693005/diff/40001/Source/core/events/Mous...
Source/core/events/MouseEvent.cpp:94: , m_button(button == -1 ? 0 : button)
On 2015/06/09 11:14:31, ramya.v wrote:
> On 2015/06/09 09:35:38, philipj wrote:
> > Getting rid of these casts will be nice. However, I think that coercing -1
to
> 0
> > is wrong for the web-exposed ways of creating a MouseEvent, and Firefox
> doesn't
> > do it in this test:
> >
http://jsconsole.com/?new%20MouseEvent%28%27click%27%2C{button%3A-1}%29.button
> > 
> > I tested IE, it doesn't have the MouseEvent contructor, but this also allows
> -1
> > to pass through:
> > var event = document.createEvent("MouseEvent");
> >
>
event.initMouseEvent('click',false,false,null,0,0,0,0,0,false,false,false,false,-1,null);
> > alert(event.button);
> > 
> > However, strangely enough, passing -2 to IE results in button 65534, just
like
> > Blink/WebKit.
> > 
> > I think the right thing to do here is to simply pass through the values as
> they
> > are, as the spec doesn't require any coercion AFAICT. You will need to check
> if
> > there are any internal uses reaching any of these code paths where (unsigned
> > short)-1 will be passed, by any name.
> > https://code.google.com/p/chromium/codesearch is a good place to dig into
that
> > kind of question.
> 
> I've checked for any internal paths which could pass value of -1. Apart form
> fakeMouseMoves from EventHandler.cpp, PopupMenuImpl::setValueAndClosePopup is
> creating default PlatformMouseEvent constructor in which default value of
button
> is -1.
> Apart from these I could not find any specific place where they are sending
> explicit value of -1.
> 
> The check (unsigned short)-1?0:button might be added because negative values
are
> not allowed to be assigned to button since it is unsigned short. Now as you
> suggested probably we can pass the values as they are similar to firefox
> behavior.

OK, it looks like the MouseButton enum member NoButton in PlatformMouseEvent.h
is the source of the -1 handling, and MouseEvent.button uses 0 to mean both
"left button" and "no button", which is also why there's an explicit
m_buttonDown member in MouseEvent, and some silly code in MouseEvent::which().

This would probably be less strange if we nuked the MouseButton in
PlatformMouseEvent and the *ButtonDown in PlatformEvent.h's Modifiers, replacing
both with

enum MouseButtons {
    NoButton = 0,
    LeftButton = 1,
    MiddleButton = 2,
    RightButton = 4
}

That matches https://w3c.github.io/uievents/#widl-MouseEvent-buttons

Then PlatformMouseEvent would have both button and buttons members in this form.
It would be the job of MouseEvent to convert this to the slightly wacky format
required to be web-exposed, where both NoButton and LeftButton are represented
by 0 in some contexts.

This is a bit invasive, so let's wait and see what Rick makes of this situation.

[Rick Byers, 2015-06-09 14:05:55]

On 2015/06/09 12:04:07, philipj wrote:
> OK, it looks like the MouseButton enum member NoButton in PlatformMouseEvent.h
> is the source of the -1 handling, and MouseEvent.button uses 0 to mean both
> "left button" and "no button", which is also why there's an explicit
> m_buttonDown member in MouseEvent, and some silly code in MouseEvent::which().

Yeah, this is why we changed button to be short - so that we could expose -1 for
"no button", at least for PointerEvent instances (but hopefully someday for
MouseEvent instances too if the breaking change is sufficiently web compatible).

In order to implement PointerEvent::button simply we'd ideally have
MouseEvent::m_button keep the -1 value.  MouseEvent::button() should be the one
place with the hack convert to 0 (and PointerEvent::button will override that to
return the value directly).  This will also let us remove
MouseEvent::m_buttonDown and the goofy logic in MouseEvent::which().

>
https://codereview.chromium.org/1165693005/diff/40001/Source/core/events/Mous...
> File Source/core/events/MouseEvent.cpp (right):
> 
>
https://codereview.chromium.org/1165693005/diff/40001/Source/core/events/Mous...
> Source/core/events/MouseEvent.cpp:94: , m_button(button == -1 ? 0 : button)
> On 2015/06/09 11:14:31, ramya.v wrote:
> > On 2015/06/09 09:35:38, philipj wrote:
> > > Getting rid of these casts will be nice. However, I think that coercing -1
> to
> > 0
> > > is wrong for the web-exposed ways of creating a MouseEvent, and Firefox
> > doesn't
> > > do it in this test:
> > >
> http://jsconsole.com/?new%20MouseEvent%28%27click%27%2C{button%3A-1}%29.button
> > > 
> > > I tested IE, it doesn't have the MouseEvent contructor, but this also
allows
> > -1
> > > to pass through:
> > > var event = document.createEvent("MouseEvent");
> > >
> >
>
event.initMouseEvent('click',false,false,null,0,0,0,0,0,false,false,false,false,-1,null);
> > > alert(event.button);
> > > 
> > > However, strangely enough, passing -2 to IE results in button 65534, just
> like
> > > Blink/WebKit.
> > > 
> > > I think the right thing to do here is to simply pass through the values as
> > they
> > > are, as the spec doesn't require any coercion AFAICT. You will need to
check
> > if
> > > there are any internal uses reaching any of these code paths where
(unsigned
> > > short)-1 will be passed, by any name.
> > > https://code.google.com/p/chromium/codesearch is a good place to dig into
> that
> > > kind of question.
> > 
> > I've checked for any internal paths which could pass value of -1. Apart form
> > fakeMouseMoves from EventHandler.cpp, PopupMenuImpl::setValueAndClosePopup
is
> > creating default PlatformMouseEvent constructor in which default value of
> button
> > is -1.
> > Apart from these I could not find any specific place where they are sending
> > explicit value of -1.
> > 
> > The check (unsigned short)-1?0:button might be added because negative values
> are
> > not allowed to be assigned to button since it is unsigned short. Now as you
> > suggested probably we can pass the values as they are similar to firefox
> > behavior.

I agree that ideally we'd pass the values through directly unmodified.  Since
this could cause a web compat issue (though seems unlikely to me), I suggest we
do that in a separate CL.  Eg. what if here we make MouseEvent::button the only
place that converts the -1 to 0 (which should keep the exposed behavior the same
as today).

Then in a follow up CL we can add some logic to relax the -1->0 conversion so it
doesn't happen for mouse events created from script.  This should be easy to do
if you wait for https://codereview.chromium.org/894913002/ to land first (which
has unfortunately been moving rather slowly).  This will mean
MouseEvent::button() is a little hacky, but that's where the hack belongs IMHO
and I suspect we'll be able to remove the compat hack entirely at some point in
the future.

WDYT?

> This would probably be less strange if we nuked the MouseButton in
> PlatformMouseEvent and the *ButtonDown in PlatformEvent.h's Modifiers,
replacing
> both with
> 
> enum MouseButtons {
>     NoButton = 0,
>     LeftButton = 1,
>     MiddleButton = 2,
>     RightButton = 4
> }
> 
> That matches https://w3c.github.io/uievents/#widl-MouseEvent-buttons
> 
> Then PlatformMouseEvent would have both button and buttons members in this
form.
> It would be the job of MouseEvent to convert this to the slightly wacky format
> required to be web-exposed, where both NoButton and LeftButton are represented
> by 0 in some contexts.
> 
> This is a bit invasive, so let's wait and see what Rick makes of this
situation.

Yeah we could do this too - I agree it would be nice to have a single format
internally.  But it's a bunch of work and I don't think it really buys us much
over the above plan.  Also it adds some risk of furthering the confusion between
the semantics of 'button' and that of 'buttons'.  In particular 'button'
represents which button changed state in this event (so can't be a bitmask)
while 'buttons' represents the current state of all buttons (eg. on mouseup
'buttons' by itself doesn't tell you what button was released).

[philipj_slow, 2015-06-09 15:27:53]

Isolating all the weirdness inside MouseEvent::button() and getting rid of
m_buttonDown sounds like a good start to me. If by the time PointerEvent.button
needs to be -1 the hack is still there, it could be made virtual I guess. Rick,
this is really your area, so if you could summarize what needs to be changed
relative to PS3 that would be great.

[Rick Byers, 2015-06-09 21:25:39]

On 2015/06/09 15:27:53, philipj wrote:
> Isolating all the weirdness inside MouseEvent::button() and getting rid of
> m_buttonDown sounds like a good start to me. If by the time
PointerEvent.button
> needs to be -1 the hack is still there, it could be made virtual I guess.
Rick,
> this is really your area, so if you could summarize what needs to be changed
> relative to PS3 that would be great.

Ok.  PS3 is close, I think you just need to do the following:
 - remove all the "button == -1 ? 0 : button" code and put it into the
MouseEvent::button function instead
 - simplify MouseEvent::which to use m_button directly and remove m_buttonDown

[Rick Byers, 2015-06-09 21:27:38]

On 2015/06/09 21:25:39, Rick Byers wrote:
> On 2015/06/09 15:27:53, philipj wrote:
> > Isolating all the weirdness inside MouseEvent::button() and getting rid of
> > m_buttonDown sounds like a good start to me. If by the time
> PointerEvent.button
> > needs to be -1 the hack is still there, it could be made virtual I guess.
> Rick,
> > this is really your area, so if you could summarize what needs to be changed
> > relative to PS3 that would be great.
> 
> Ok.  PS3 is close, I think you just need to do the following:
>  - remove all the "button == -1 ? 0 : button" code and put it into the
> MouseEvent::button function instead
>  - simplify MouseEvent::which to use m_button directly and remove m_buttonDown

Sorry, and also update PlatformTouchPoint to use 'int' as well and do a quick
scan to see if there's any other consumers of the relevant APIs using 'unsigned'
for the ID (probably nothing important but worth a quick check).

[ramya.v, 2015-06-10 07:17:04]

Updated as per review comments. PTAL!

Thanks

[philipj_slow, 2015-06-10 08:46:25]

https://codereview.chromium.org/1165693005/diff/80001/LayoutTests/fast/events...
File LayoutTests/fast/events/constructors/mouse-event-constructor.html (left):

https://codereview.chromium.org/1165693005/diff/80001/LayoutTests/fast/events...
LayoutTests/fast/events/constructors/mouse-event-constructor.html:109:
shouldBe("new MouseEvent('eventType', { button: 65534 }).button", "65534");
Why not keep this test to show the new behavior? I guess it's -2?

https://codereview.chromium.org/1165693005/diff/80001/LayoutTests/fast/events...
File LayoutTests/fast/events/constructors/mouse-event-constructor.html (right):

https://codereview.chromium.org/1165693005/diff/80001/LayoutTests/fast/events...
LayoutTests/fast/events/constructors/mouse-event-constructor.html:110: //
Numbers that are equal to -1 should be treated as 0.
Can you also test -2, to show that it's just -1 and not all negative values that
are coerced?

https://codereview.chromium.org/1165693005/diff/80001/LayoutTests/fast/events...
File LayoutTests/fast/events/constructors/wheel-event-constructor.html (left):

https://codereview.chromium.org/1165693005/diff/80001/LayoutTests/fast/events...
LayoutTests/fast/events/constructors/wheel-event-constructor.html:176:
shouldBe("new WheelEvent('eventType', { button: 65534 }).button", "65534");
Ditto.

https://codereview.chromium.org/1165693005/diff/80001/Source/core/events/Mous...
File Source/core/events/MouseEvent.cpp (right):

https://codereview.chromium.org/1165693005/diff/80001/Source/core/events/Mous...
Source/core/events/MouseEvent.cpp:96: , m_buttonDown(button != -1)
Can the m_buttonDown member be removed? Just implement it has m_button != -1

[ramya.v, 2015-06-10 09:21:28]

Updated test cases as per review comments. PTAL!

https://codereview.chromium.org/1165693005/diff/80001/LayoutTests/fast/events...
File LayoutTests/fast/events/constructors/mouse-event-constructor.html (left):

https://codereview.chromium.org/1165693005/diff/80001/LayoutTests/fast/events...
LayoutTests/fast/events/constructors/mouse-event-constructor.html:109:
shouldBe("new MouseEvent('eventType', { button: 65534 }).button", "65534");
On 2015/06/10 08:46:25, philipj wrote:
> Why not keep this test to show the new behavior? I guess it's -2?

Done.

https://codereview.chromium.org/1165693005/diff/80001/LayoutTests/fast/events...
File LayoutTests/fast/events/constructors/mouse-event-constructor.html (right):

https://codereview.chromium.org/1165693005/diff/80001/LayoutTests/fast/events...
LayoutTests/fast/events/constructors/mouse-event-constructor.html:110: //
Numbers that are equal to -1 should be treated as 0.
On 2015/06/10 08:46:25, philipj wrote:
> Can you also test -2, to show that it's just -1 and not all negative values
that
> are coerced?

Done.

https://codereview.chromium.org/1165693005/diff/80001/LayoutTests/fast/events...
File LayoutTests/fast/events/constructors/wheel-event-constructor.html (left):

https://codereview.chromium.org/1165693005/diff/80001/LayoutTests/fast/events...
LayoutTests/fast/events/constructors/wheel-event-constructor.html:176:
shouldBe("new WheelEvent('eventType', { button: 65534 }).button", "65534");
On 2015/06/10 08:46:25, philipj wrote:
> Ditto.

Done.

https://codereview.chromium.org/1165693005/diff/80001/Source/core/events/Mous...
File Source/core/events/MouseEvent.cpp (right):

https://codereview.chromium.org/1165693005/diff/80001/Source/core/events/Mous...
Source/core/events/MouseEvent.cpp:96: , m_buttonDown(button != -1)
On 2015/06/10 08:46:25, philipj wrote:
> Can the m_buttonDown member be removed? Just implement it has m_button != -1

buttonDown() function is used in other places like
HTMLSelectElement::listBoxDefaultEventHandler,
WebMouseEventBuilder::WebMouseEventBuilder, etc.
m_button is private, and button() returns 0 in place of -1. So it may not be
possible now to remove m_buttonDown variable completely.

[ramya.v, 2015-06-10 09:25:39]

https://codereview.chromium.org/1165693005/diff/80001/Source/core/events/Mous...
File Source/core/events/MouseEvent.cpp (right):

https://codereview.chromium.org/1165693005/diff/80001/Source/core/events/Mous...
Source/core/events/MouseEvent.cpp:96: , m_buttonDown(button != -1)
On 2015/06/10 09:21:28, ramya.v wrote:
> On 2015/06/10 08:46:25, philipj wrote:
> > Can the m_buttonDown member be removed? Just implement it has m_button != -1
> 
> buttonDown() function is used in other places like
> HTMLSelectElement::listBoxDefaultEventHandler,
> WebMouseEventBuilder::WebMouseEventBuilder, etc.
> m_button is private, and button() returns 0 in place of -1. So it may not be
> possible now to remove m_buttonDown variable completely.


Sorry, didn't get you,  yes the variable can be removed. In buttonDown()
function will use m_button != -1

[ramya.v, 2015-06-10 09:41:25]

PTAL!

https://codereview.chromium.org/1165693005/diff/80001/Source/core/events/Mous...
File Source/core/events/MouseEvent.cpp (right):

https://codereview.chromium.org/1165693005/diff/80001/Source/core/events/Mous...
Source/core/events/MouseEvent.cpp:96: , m_buttonDown(button != -1)
On 2015/06/10 08:46:25, philipj wrote:
> Can the m_buttonDown member be removed? Just implement it has m_button != -1

Done.

[philipj_slow, 2015-06-10 13:54:46]

Code looks good, just some tiny changes to the test needed. I say LGTM, but
please let rbyers@ have the last say as he's our master of input matters :)

https://codereview.chromium.org/1165693005/diff/120001/LayoutTests/fast/event...
File LayoutTests/fast/events/constructors/mouse-event-constructor.html (right):

https://codereview.chromium.org/1165693005/diff/120001/LayoutTests/fast/event...
LayoutTests/fast/events/constructors/mouse-event-constructor.html:109:
shouldBe("new MouseEvent('eventType', { button: 65534 }).button", "-2");
Oh, I guess you should actually move this to the "Numbers out of the short
range". The range of short is [-32768, 32765] so can you make sure to test those
boundary points as well?

https://codereview.chromium.org/1165693005/diff/120001/LayoutTests/fast/event...
LayoutTests/fast/events/constructors/mouse-event-constructor.html:139: //
Numbers within the short range.
Oops, buttons is still unsigned short, change this and the below back :)

[Rick Byers, 2015-06-10 15:23:14]

On 2015/06/09 21:27:38, Rick Byers wrote:
> On 2015/06/09 21:25:39, Rick Byers wrote:
> > On 2015/06/09 15:27:53, philipj wrote:
> > > Isolating all the weirdness inside MouseEvent::button() and getting rid of
> > > m_buttonDown sounds like a good start to me. If by the time
> > PointerEvent.button
> > > needs to be -1 the hack is still there, it could be made virtual I guess.
> > Rick,
> > > this is really your area, so if you could summarize what needs to be
changed
> > > relative to PS3 that would be great.
> > 
> > Ok.  PS3 is close, I think you just need to do the following:
> >  - remove all the "button == -1 ? 0 : button" code and put it into the
> > MouseEvent::button function instead
> >  - simplify MouseEvent::which to use m_button directly and remove
m_buttonDown
> 
> Sorry, and also update PlatformTouchPoint to use 'int' as well and do a quick
> scan to see if there's any other consumers of the relevant APIs using
'unsigned'
> for the ID (probably nothing important but worth a quick check).

Damn, I obviously confused two different (but similar) reviews with this comment
- sorry!  (https://codereview.chromium.org/1170703003/)

[Rick Byers, 2015-06-10 15:26:48]

On 2015/06/10 15:23:14, Rick Byers wrote:
> On 2015/06/09 21:27:38, Rick Byers wrote:
> > On 2015/06/09 21:25:39, Rick Byers wrote:
> > > On 2015/06/09 15:27:53, philipj wrote:
> > > > Isolating all the weirdness inside MouseEvent::button() and getting rid
of
> > > > m_buttonDown sounds like a good start to me. If by the time
> > > PointerEvent.button
> > > > needs to be -1 the hack is still there, it could be made virtual I
guess.
> > > Rick,
> > > > this is really your area, so if you could summarize what needs to be
> changed
> > > > relative to PS3 that would be great.
> > > 
> > > Ok.  PS3 is close, I think you just need to do the following:
> > >  - remove all the "button == -1 ? 0 : button" code and put it into the
> > > MouseEvent::button function instead
> > >  - simplify MouseEvent::which to use m_button directly and remove
> m_buttonDown
> > 
> > Sorry, and also update PlatformTouchPoint to use 'int' as well and do a
quick
> > scan to see if there's any other consumers of the relevant APIs using
> 'unsigned'
> > for the ID (probably nothing important but worth a quick check).
> 
> Damn, I obviously confused two different (but similar) reviews with this
comment
> - sorry!  (https://codereview.chromium.org/1170703003/)

This looks great to me.  LGTM modulo the test improvements Philip suggested.
Big picture, the relevant things to test are just these:
 - values outside 'short' range get wrapped
 - -1 is handled specially as 0
 - some other negative value gets preserved

[ramya.v, 2015-06-11 05:02:14]

Updated as per review comments. PTAL!

Thanks

https://codereview.chromium.org/1165693005/diff/120001/LayoutTests/fast/event...
File LayoutTests/fast/events/constructors/mouse-event-constructor.html (right):

https://codereview.chromium.org/1165693005/diff/120001/LayoutTests/fast/event...
LayoutTests/fast/events/constructors/mouse-event-constructor.html:109:
shouldBe("new MouseEvent('eventType', { button: 65534 }).button", "-2");
On 2015/06/10 13:54:46, philipj wrote:
> Oh, I guess you should actually move this to the "Numbers out of the short
> range". The range of short is [-32768, 32765] so can you make sure to test
those
> boundary points as well?

Done.

https://codereview.chromium.org/1165693005/diff/120001/LayoutTests/fast/event...
LayoutTests/fast/events/constructors/mouse-event-constructor.html:139: //
Numbers within the short range.
On 2015/06/10 13:54:46, philipj wrote:
> Oops, buttons is still unsigned short, change this and the below back :)

Done.

[philipj_slow, 2015-06-11 08:27:28]

The CQ bit was checked by philipj@opera.com

[philipj_slow, 2015-06-11 08:27:29]

lgtm

Nice comprehensive tests, thanks!

[philipj_slow, 2015-06-11 08:27:29]

The patchset sent to the CQ was uploaded after l-g-t-m from rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/1165693005/#ps160001
(title: " ")

[commit-bot: I haz the power, 2015-06-11 08:27:53]

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

[commit-bot: I haz the power, 2015-06-11 09:41:22]

Committed patchset #9 (id:160001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196932