230651 - don't create new rule processors when enabling/disabling a style sheet

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Brian Ryner (not reading)]

We can eliminate the style set clearing its rule processors array (and
subsequent creation of new rule processors) for enabling/disabling style sheets
by calling ClearRuleCascades() from CSSStyleSheetImpl::SetEnabled().

Comment 1

[Brian Ryner (not reading)]

Attachment: patch

Comment 2

[Brian Ryner (not reading)]

Comment on attachment 138826 [details] [diff] [review]
patch

dbaron, can you r+sr?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Actually, if |mInner && mInner->mComplete| there is no need to clear the
cascades -- incomplete sheets won't be affecting anyone's style anyway.

With this change, is there still good reason to move sheets in and out of the
style set as their enabled state changes?  I guess with alternate sheets that's
still a win?

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

In what way do we move sheets in and out of the style set as their enabled state
changes?

Comment 5

[Boris Zbarsky [:bzbarsky]]

See nsDocument::SetStyleSheetApplicableState
(http://lxr.mozilla.org/seamonkey/source/content/base/src/nsDocument.cpp#1500)

Also see the checks done in nsDocument::AddStyleSheet,
sDocument::InsertStyleSheetAt, and nsDocument::UpdateStyleSheets to not put
sheets that are not applicable in style sets (all those used to be just disabled
checks back before we had a concept of "applicable").

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 138826 [details] [diff] [review]
patch

sr=bzbarsky if the first paragraph of comment 3 is addressed.

Comment 7

[Brian Ryner (not reading)]

checked in.

Comment 8

[Boris Zbarsky [:bzbarsky]]

I just realized.. this changed the SetEnabled code, but not SetDisabled.  That
needs to be changed too.

Reopening to make sure that happens.

Comment 9

[Brian Ryner (not reading)]

Attachment: er, yeah

how about this

(note, i'm using explicit scope resolution to avoid the second function call
being virtual)

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 139054 [details] [diff] [review]
er, yeah

Looks reasonable (though in my tree the functions are subtly different, but I
can merge).

Comment 11

[Brian Ryner (not reading)]

Checked in.