Ignore should-scroll-on-main-thread if main frame is not scrollable

Ignore should-scroll-on-main-thread if main frame is not scrollable

This patch was originally reverted because the call to FrameView::isScrollable
that was added could crash if the body had a NULL style. This patch adds a
fix handle that scenario, and a test that reproduces it.

This reverts commit 0e53aff60891596f88df001ef3d91422d83cc560.
(Review URL: https://chromiumcodereview.appspot.com/23477042)

And relands commit 1a3edc542e31626a9b46ae2b18eb227c7031f3d6
(Review URL: https://chromiumcodereview.appspot.com/22694003)

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

[jamesr, 2013-08-30 19:06:55]

Do you know which of these is NULL and why?  Where's your test?

[Ian Vollick, 2013-09-11 19:54:10]

On 2013/08/30 19:06:55, jamesr wrote:
> Do you know which of these is NULL and why?  Where's your test?

Turns out it's the body's style. I've constructed a test that hits this codepath
every time. PTAL.

[jamesr, 2013-09-11 20:33:16]

code lgtm, but the patch description and tests both need work

https://codereview.chromium.org/23641004/diff/5001/LayoutTests/compositing/ov...
File
LayoutTests/compositing/overflow/do-not-crash-when-checking-frame-view-is-scrollable-expected.txt
(right):

https://codereview.chromium.org/23641004/diff/5001/LayoutTests/compositing/ov...
LayoutTests/compositing/overflow/do-not-crash-when-checking-frame-view-is-scrollable-expected.txt:1:
PASS Did not crash
this is not not necessary - if the test crashes it fails regardless of the text
output

https://codereview.chromium.org/23641004/diff/5001/LayoutTests/compositing/ov...
File
LayoutTests/compositing/overflow/ignore-main-thread-scroll-reasons-when-main-frame-not-scrollable.html
(right):

https://codereview.chromium.org/23641004/diff/5001/LayoutTests/compositing/ov...
LayoutTests/compositing/overflow/ignore-main-thread-scroll-reasons-when-main-frame-not-scrollable.html:62:
// (Barely) modified version of shouldBeUndefined from js-test-pre.js.
why do you need all this? don't you just want "reasons === undefined" on line
85? copy-paste makes for sad engineers :(

[Ian Vollick, 2013-09-11 22:50:17]

I've updated the tests and the description. Is this better?

[jamesr, 2013-09-11 22:54:13]

Tests lgtm.  The patch description still has some issues:

1.) The subject is not all that informative.  Say what the patch does in one
line, having "Revert Revert" is noise.  Put history in the description
2.) The first line of the patch description should be the same as the subject.

Feel free to land after fixing those

[commit-bot: I haz the power, 2013-09-11 22:59:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/23641004/11001

[commit-bot: I haz the power, 2013-09-12 00:38:33]

Change committed as 157644