Add a missing null check in SliderThumbElement::stopDragging()

Add a missing null check in SliderThumbElement::stopDragging()

hostInput() is null checked in several other SliderThumbElement
functions and clearly can return null, so just handle it.

Add GCController.collect() to the test case to make the crash trigger
reliably, which was necessary in order to debug it.

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

[philipj_slow, 2014-03-25 18:03:00]

PTAL, and see the comments in the bug.

[acolwell GONE FROM CHROMIUM, 2014-03-25 22:28:57]

lgtm % nits

https://codereview.chromium.org/211403003/diff/1/LayoutTests/media/audio-dele...
File LayoutTests/media/audio-delete-while-slider-thumb-clicked.html (right):

https://codereview.chromium.org/211403003/diff/1/LayoutTests/media/audio-dele...
LayoutTests/media/audio-delete-while-slider-thumb-clicked.html:21: if
(window.GCController)
nit: I think you should move this into deleteAudio() since this clearly isn't a
button click and it's a little confusing to have it here. Adding a comment about
why you GC in some testing environments and force a click in others would be
nice.

https://codereview.chromium.org/211403003/diff/1/Source/core/html/shadow/Slid...
File Source/core/html/shadow/SliderThumbElement.cpp (right):

https://codereview.chromium.org/211403003/diff/1/Source/core/html/shadow/Slid...
Source/core/html/shadow/SliderThumbElement.cpp:229: return
hostInput()->matchesReadOnlyPseudoClass();
nit: Should this and the one below be protected as well?

[philipj_slow, 2014-03-26 08:15:31]

https://codereview.chromium.org/211403003/diff/1/LayoutTests/media/audio-dele...
File LayoutTests/media/audio-delete-while-slider-thumb-clicked.html (right):

https://codereview.chromium.org/211403003/diff/1/LayoutTests/media/audio-dele...
LayoutTests/media/audio-delete-while-slider-thumb-clicked.html:21: if
(window.GCController)
On 2014/03/25 22:28:57, acolwell wrote:
> nit: I think you should move this into deleteAudio() since this clearly isn't
a
> button click and it's a little confusing to have it here. Adding a comment
about
> why you GC in some testing environments and force a click in others would be
> nice.

Ahem... this code was copied from adopt-node-crash.html, but I messed up by
copying the return as well, fortunately you didn't miss it!

I didn't put it in deleteAudio() because that's run in an event listener for the
audio element, and the audio element cannot be collected at that point, at least
not in my testing.

I'll fix this by copying all of forceGC() instead, which should make things
clearer.

https://codereview.chromium.org/211403003/diff/1/Source/core/html/shadow/Slid...
File Source/core/html/shadow/SliderThumbElement.cpp (right):

https://codereview.chromium.org/211403003/diff/1/Source/core/html/shadow/Slid...
Source/core/html/shadow/SliderThumbElement.cpp:229: return
hostInput()->matchesReadOnlyPseudoClass();
On 2014/03/25 22:28:57, acolwell wrote:
> nit: Should this and the one below be protected as well?

These are only called from SelectorChecker.cpp. Probably it can never be null as
the code is now, but these are overrides for Element just like
isDisabledFormControl() so I guess null checks are more consistent and harmless
in any case.

The only remaining unchecked use is the hostInput() in
SliderThumbElement::setPositionFromPoint but all the callers that I can see
check isDisabledFormControl() in some way or another.

[philipj_slow, 2014-03-26 08:17:13]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-26 08:17:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/211403003/80001

[commit-bot: I haz the power, 2014-03-26 08:25:23]

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

[commit-bot: I haz the power, 2014-03-26 08:25:23]

Try jobs failed on following builders:
  tryserver.blink on win_blink_compile_dbg

[philipj_slow, 2014-03-26 08:28:16]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-26 08:28:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/211403003/80001

[commit-bot: I haz the power, 2014-03-26 09:05:12]

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

[commit-bot: I haz the power, 2014-03-26 09:05:12]

Try jobs failed on following builders:
  tryserver.blink on win_blink_rel

[philipj_slow, 2014-03-26 09:07:29]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-26 09:07:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/211403003/80001

[commit-bot: I haz the power, 2014-03-26 09:24:26]

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

[commit-bot: I haz the power, 2014-03-26 09:24:26]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[philipj_slow, 2014-03-26 10:04:43]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-26 10:04:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/211403003/80001

[commit-bot: I haz the power, 2014-03-26 11:07:34]

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

[commit-bot: I haz the power, 2014-03-26 11:07:34]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[philipj_slow, 2014-03-26 11:30:53]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-26 11:30:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/211403003/80001

[commit-bot: I haz the power, 2014-03-26 12:34:29]

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

[commit-bot: I haz the power, 2014-03-26 12:34:30]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[philipj_slow, 2014-03-26 13:19:08]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-26 13:19:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/211403003/80001

[commit-bot: I haz the power, 2014-03-26 14:28:35]

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

[commit-bot: I haz the power, 2014-03-26 14:28:35]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[philipj_slow, 2014-03-26 18:28:53]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-26 18:29:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/211403003/80001

[commit-bot: I haz the power, 2014-03-26 19:37:30]

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

[commit-bot: I haz the power, 2014-03-26 19:37:31]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[philipj_slow, 2014-03-27 02:03:01]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-27 02:03:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/211403003/330001

[commit-bot: I haz the power, 2014-03-27 03:12:39]

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

[commit-bot: I haz the power, 2014-03-27 03:12:39]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[philipj_slow, 2014-03-27 04:17:17]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2014-03-27 04:17:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/211403003/330001

[commit-bot: I haz the power, 2014-03-27 05:25:56]

Change committed as 170139