Don't repaint button when its click() method is called

Don't repaint button when its click() method is called

When click() method is called, previously we change the active status
of the element to true and then back to false. This doesn't change
anything for a button (or other elements that status doesn't change
after click()) but causes a repaint.

Don't change active status when click() method is called. (Elements
that status changes after click() call (e.g. checkboxes) will still
repaint.)

This doesn't change how we handle real mouse down/up/click events.

BUG=369358
TEST=fast/repaint/button-checkbox-click-method-repaint.html
TEST=fast/events/button-mouse-active.html # ensures :active is correct on real mouse down events.

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

[Xianzhu, 2014-05-20 21:06:39]

PTAL. fast/events/label-click.html is separated into
https://codereview.chromium.org/291243002/.

[Xianzhu, 2014-05-27 16:19:03]

ping...

[leviw_travelin_and_unemployed, 2014-05-27 17:51:34]

lgtm

https://codereview.chromium.org/291563009/diff/20001/LayoutTests/fast/repaint...
File LayoutTests/fast/repaint/button-checkbox-click-method-repaint-expected.txt
(right):

https://codereview.chromium.org/291563009/diff/20001/LayoutTests/fast/repaint...
LayoutTests/fast/repaint/button-checkbox-click-method-repaint-expected.txt:9:
(rect 74.00 13.00 13.00 13.00)
I presume this rect is the checkbox?

[Xianzhu, 2014-05-27 18:25:42]

https://codereview.chromium.org/291563009/diff/20001/LayoutTests/fast/repaint...
File LayoutTests/fast/repaint/button-checkbox-click-method-repaint-expected.txt
(right):

https://codereview.chromium.org/291563009/diff/20001/LayoutTests/fast/repaint...
LayoutTests/fast/repaint/button-checkbox-click-method-repaint-expected.txt:9:
(rect 74.00 13.00 13.00 13.00)
On 2014/05/27 17:51:34, leviw wrote:
> I presume this rect is the checkbox?

Yes.

[Xianzhu, 2014-05-27 18:36:39]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2014-05-27 18:38:12]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/291563009/40001

[commit-bot: I haz the power, 2014-05-27 20:30:42]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/9858)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/9676)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/9132)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/9323)

[commit-bot: I haz the power, 2014-05-27 21:11:07]

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

[commit-bot: I haz the power, 2014-05-27 21:11:08]

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

[Xianzhu, 2014-05-27 23:01:47]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2014-05-27 23:02:06]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/291563009/60001

[esprehn, 2014-05-27 23:11:46]

The CQ bit was unchecked by esprehn@chromium.org

[esprehn, 2014-05-27 23:12:34]

This is web observable, if you do querySelector(":active") inside the click
handler you can see which element is active.

What do other browsers do?

[Xianzhu, 2014-05-27 23:29:06]

On 2014/05/27 23:12:34, esprehn wrote:
> This is web observable, if you do querySelector(":active") inside the click
> handler you can see which element is active.
> 
> What do other browsers do?

The original sequence is:
 1. dispatch mouseover event if mouseEventOptions == SendMouseOverUpDownEvents
 2. dispatch mousedown event if mouseEventOptions != SendNoEvents
 3. setActive(true)
 4. dispatch mouseup event if mouseEventOptions != SendNoEvents
 5. setActive(false)
 6. dispatch click event

When click() method is called, the mouseEventOptions parameter of
EventDispatcher::dispatchSimulatedClick() is SendNoEvents, so there are only 3,
5 and 6. As click event (6) is sent after setActive(false), the JS event handler
will always get non-active state.

However we should setActive when we send mousedown and mouseup events to ensure
:active state is unaffected in mouseup event handler.

Will upload a new patch soon.

[Xianzhu, 2014-05-27 23:37:45]

Done. PTAL.

[Xianzhu, 2014-05-28 16:29:00]

I'm CQing as I believe esprehn@'s concern has been addressed :)

[Xianzhu, 2014-05-28 16:29:09]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2014-05-28 16:29:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/291563009/80001

[Xianzhu, 2014-05-28 18:33:27]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2014-05-28 18:33:56]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/291563009/100001

[commit-bot: I haz the power, 2014-05-28 20:31:09]

Change committed as 174988