1204535 - Refactor computation of mShouldBuildScrollableLayer

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

While looking at how to fix bug 1195431 I noticed that the code to compute mShouldBuildScrollableLayer is very convoluted. There's a number of other cleanups that can be done around that code as well, so I'm filing this bug for it.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

My WIPs are the top few commits at https://github.com/staktrace/gecko-dev/commits/refactor

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Well the refactoring didn't end up quite where I wanted but there's some patches which I think are good to land. Try push for them is at https://treeherder.mozilla.org/#/jobs?repo=try&revision=df1a8fe4247c

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 1 - Delete a no-op hunk of code

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 2 - Flatten conditional structure one level

This makes the refactoring in part 3 more obvious.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 3 - Collapse redundant conditional block

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 4 - Rename usingDisplayport to usingDisplayPort

At whistler we did a grep and found more usages of "displayPort" than "displayport" and decided to move towards that. This function has both usingDisplayport and usingDisplayPort which could get confusing, so using the same name is a good idea.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 5 - Relocate a comment

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 6 - Hoist the initial computation of usingDisplayPort

Comment 9

[Timothy Nikkel (:tnikkel)]

Thanks. I had patches to do most of this as part of an even larger refactor (that didn't get finished).

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Thanks for the speedy reviews! I hope these patches don't cause you too much rebase pain on the rest of the refactoring you had.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9d08ea4e6be9
https://hg.mozilla.org/integration/mozilla-inbound/rev/03c1c7c1e144
https://hg.mozilla.org/integration/mozilla-inbound/rev/c12eaa3668ea
https://hg.mozilla.org/integration/mozilla-inbound/rev/c60510f350b1
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0f0bf01b3a8
https://hg.mozilla.org/integration/mozilla-inbound/rev/8d05eefc5da2

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9d08ea4e6be9
https://hg.mozilla.org/mozilla-central/rev/03c1c7c1e144
https://hg.mozilla.org/mozilla-central/rev/c12eaa3668ea
https://hg.mozilla.org/mozilla-central/rev/c60510f350b1
https://hg.mozilla.org/mozilla-central/rev/f0f0bf01b3a8
https://hg.mozilla.org/mozilla-central/rev/8d05eefc5da2