201300 - Fix dispatching of AttributeChanges to nsGfxScrollFrame

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Currently every nsGfxScrollFrame registers a document listener so that it can
see changes to the 'curpos' attribute on both of its scrollbars. This is way
overkill and could be hurting performance a lot in some cases.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

My approach to fixing this is to have nsScrollbarFrame forward its frame
attribute change notification to its nsGfxScrollFrame parent (if there is one),
if the attribute is 'curpos'. This eliminates the need for nsGfxScrollFrame to
observe the document. It reduces the number of attribute change notifications to
nsGfxScrollFrame to *zero* for a sequence of normal (non-scrolling) page loads.
Patch coming up.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 119937 [details] [diff] [review]
fix

wrong patch, sorry

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 119938 [details] [diff] [review]
fix

>+  // This gets called when the 'curpos' attribute on one of the scrollbars changes
>+  nsresult CurPosAttributeChanged(nsIPresContext* aPresContext,

Might be nice to make this protected and make nsGfxScrollFrame a friend
class....

>+  parent->QueryInterface(NS_GET_IID(nsIScrollableFrame), (void**)&scrollable);

rv = CallQueryInterface(parent, &scrollable);

r/sr=me with those.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

> Might be nice to make this protected and make nsGfxScrollFrame a friend
> class....

nsGfxScrollFrameInner is only visible in nsGfxScrollFrame.cpp, so this doesn't
seem necessary. (Hence "Inner")

> rv = CallQueryInterface(parent, &scrollable);

Oh yeah. willdo

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked in. This should be interesting.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Hmm. Tp may have gone down by some "in the noise" amount, but that's all. Well
at least we know the Tp slowdown from smooth scrolling was not due to
AttributeChanged callbacks.

Comment 9

[noririty]

Has this anything to do with bug 201435?