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

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

The reasons for forcing main frame scrolling are applicable for main frame
scrolling. If we are not scrolling the main frame (and scrolling cannot bubble
to the main frame), we should ignore this hint.

BUG=254435

[aelias_OOO_until_Jul13, 2013-08-08 05:22:05]

Hi, drive-by since I was looking at similar stuff today in the context of
http://crbug.com/244524 .  Are you trying to solve the same problem I describe
there by any chance?

[Ian Vollick, 2013-08-09 14:30:58]

On 2013/08/08 05:22:05, aelias wrote:
> Hi, drive-by since I was looking at similar stuff today in the context of
> http://crbug.com/244524 .  Are you trying to solve the same problem I describe
> there by any chance?

I am, yes, but I think our solutions complement each other nicely. Your solution
prevented a particular I-should-scroll-on-main-thread reason, which I think is
great. But at a higher level, these reasons only apply to main frame scrolling,
so ignoring them when the main frame cannot scroll is, I think, the right thing
to do and will protect us in the future if other should-scroll-on-main-thread
reasons are set.

[aelias_OOO_until_Jul13, 2013-08-09 18:44:02]

Blink also knows that the main frame isn't scrollable, so could we just apply
this policy as an additional condition in
ScrollingCoordinator::mainThreadScrollingReasons ?  I think the logic is a
better fit there than in CC.  And I don't expect new
should-scroll-on-main-thread reasons in the first place (if only because code
reviewers will push back hard against them).

[Ian Vollick, 2013-08-09 18:52:16]

On 2013/08/09 18:44:02, aelias wrote:
> Blink also knows that the main frame isn't scrollable, so could we just apply
> this policy as an additional condition in
> ScrollingCoordinator::mainThreadScrollingReasons ?  I think the logic is a
> better fit there than in CC.  And I don't expect new
> should-scroll-on-main-thread reasons in the first place (if only because code
> reviewers will push back hard against them).

Oh, no, I don't mean new reasons being added to the list. I mean we end up in
another situation (that isn't gmail) where we're legitimately setting one of the
already-in-existence should-scroll-on-main-thread reasons.

In any case, I agree that we could move the logic to blink. I'll give that a
whirl.

[Ian Vollick, 2013-08-09 19:37:27]

On 2013/08/09 18:52:16, vollick wrote:
> On 2013/08/09 18:44:02, aelias wrote:
> > Blink also knows that the main frame isn't scrollable, so could we just
apply
> > this policy as an additional condition in
> > ScrollingCoordinator::mainThreadScrollingReasons ?  I think the logic is a
> > better fit there than in CC.  And I don't expect new
> > should-scroll-on-main-thread reasons in the first place (if only because
code
> > reviewers will push back hard against them).
> 
> Oh, no, I don't mean new reasons being added to the list. I mean we end up in
> another situation (that isn't gmail) where we're legitimately setting one of
the
> already-in-existence should-scroll-on-main-thread reasons.
> 
> In any case, I agree that we could move the logic to blink. I'll give that a
> whirl.

Confirmed that this fixes this issue, and it sure feels like a nicer solution --
thanks! wdyt, Alex?

[jamesr, 2013-08-09 19:38:49]

This patch claims the base URL is svn://svn.chromium.org/chrome/trunk/src and
the patch is to Source/...

I really doubt that's the case

[Ian Vollick, 2013-08-09 19:43:00]

On 2013/08/09 19:38:49, jamesr wrote:
> This patch claims the base URL is svn://svn.chromium.org/chrome/trunk/src and
> the patch is to Source/...
> 
> I really doubt that's the case

I was naughty and reused the issue for the chromium patch for this blink patch.
That maybe confused things? I'll close this and create a new issue for the blink
patch.