Bug 14742 – Document::recalcStyle(Force) called for every updateStyleIgnorePendingStylesheets while waiting for stylesheets

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

If stylesheets have not yet loaded, updateStyleIgnorePendingStylesheets() will call updateStyleSelector() which calls recalcStyle(Force).

This is *not* a regression from the m_didCalculateStyleSelector patch (which fixed a Yahoo! Mail crash), but still would be nice to fix at some point. Basically you need updateStyleSelector() to only set a flag and postpone the update until later when you need it.

Created attachment 15656 [details]
Patch (not for now)

See also bug 14839.

[David Kilzer (:ddkilzer)]

*** Bug 14839 has been marked as a duplicate of this bug. ***

[David Kilzer (:ddkilzer)]

<rdar://problem/5376306>

(In reply to comment #3)
> *** Bug 14839 has been marked as a duplicate of this bug. ***

Raising priority to P2 since the bug affects a popular website. Dave, could you check if this is a regression from the Safari/WebKit that shipped with Tiger?

[David Kilzer (:ddkilzer)]

(In reply to comment #5)
> Raising priority to P2 since the bug affects a popular website. Dave, could you
> check if this is a regression from the Safari/WebKit that shipped with Tiger?

It is a slight regression for the Safari 3 Public Beta v. 3.0.3 on Tiger:

Using a 1.5 GHz PowerBook G4:

Using Mac OS X 10.4.10 (8R218) Tiger:
- Safari 2.0.4 (419.3) with its original WebKit:  ~8 seconds
- Safari 3 Public Beta 3.0.3 (522.12.1) with its original WebKit: ~10 seconds

However, Safari on Leopard is slightly better than Safari 2.0.4 on Tiger.

[David Kilzer (:ddkilzer)]

On the same hardware on Mac OS X 10.4.10 (8R218) using Safari 3 Public Beta v. 3.0.3 (522.12.1) with a DEBUG BUILD of ToT Webkit r24803, the page loads in ~2 seconds.

Testing methodology:

1. Launch Safari/WebKit.
2. Open the page once (untimed).
3. Hit Cmd-R and time the load.
4. Repeat Step 3 five or six times to get a ballpark figure.

(In reply to comment #7)
> On the same hardware on Mac OS X 10.4.10 (8R218) using Safari 3 Public Beta v.
> 3.0.3 (522.12.1) with a DEBUG BUILD of ToT Webkit r24803, the page loads in ~2
> seconds.

Just to make sure I understood, a debug build of r24803 is almost 5 times faster than the beta?

Created attachment 15818 [details]
Patch (for now)

This is a very pinpointed and conservative fix.

I think in the long run, Document::updateStyleSelector() should be replaced with a function that just sets a "style selector dirty" flag, and Document::styleSelector() should lazily update the style selector as needed. That is worth doing even if JavaScript becomes interruptible, because it will coalesce style updates.

[David Kilzer (:ddkilzer)]

(In reply to comment #8)
> (In reply to comment #7)
> > On the same hardware on Mac OS X 10.4.10 (8R218) using Safari 3 Public Beta v.
> > 3.0.3 (522.12.1) with a DEBUG BUILD of ToT Webkit r24803, the page loads in ~2
> > seconds.
> 
> Just to make sure I understood, a debug build of r24803 is almost 5 times
> faster than the beta?

Correct.

[Dave Hyatt]

Comment on attachment 15818 [details]
Patch (for now)

r=me

[Mark Rowe (bdash)]

Landed in r24973.