183299 - [FIXr]CreateSheet/LoadSheet issue with mIsSyncLoad

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

Just thought about this some more and we have a slight problem if someone tries
to use a single loader to load the same sheet as a UA sheet synchronously and as
any sort of sheet asynchronously.  CreateSheet will find the existing load and
clone the sheet, then LoadSheet will synchronously parse into it.

The solution, methinks, is to tell CreateSheet whether the load is sync...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: This should do the job....

This patch has tiny bits of the bug 182124 patch in it...

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 108455 [details] [diff] [review]
This should do the job....

r=sicking

However i'm not compleatly happy with the way the |if(!sheet)| tests are being
nested. I dislike code that is too deeply nested since it's hard to read, but
if you think it's worth the extra saved cycles then i won't hold the review for
that.

(don't forget to take the 182124 parts out of there before landing)

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I wonder if another way to fix this would be to merge the two checks for
"existing stylesheets" that we have. I.e. that both ::CreateSheet and
::LoadSheet checks if we have an existing load?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, merging it would work too.  I still have not found a satisfactory way to
do that (the fact that inline sheets do not go through LoadSheet does not help;
but they also don't get cached so maybe I could just make a copy of the "create
a brand new sheet" code in LoadInlineSheet....).

Comment 5

[Asa Dotzler [:asa]]

Comment on attachment 108455 [details] [diff] [review]
This should do the job....

We're trying to wrap up 1.3a and this doesn't look like a blocker. Please hold
until we open up for beta (soon). Thanks

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: de-tangled patch

Comment 7

[Boris Zbarsky [:bzbarsky]]

fixed.