Bug 9674 - REGRESSION (r15075): Blank or incomplete pages on digg.com 
Summary: REGRESSION (r15075): Blank or incomplete pages on digg.com
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Major
Assignee: Nobody
URL:
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2006-07-01 03:39 PDT by mitz
Modified: 2006-07-02 04:34 PDT (History)
2 users (show)

See Also:


Attachments
Replace the requestingScript state bit with a count (7.92 KB, patch)
2006-07-01 03:53 PDT, mitz
darin: review-
Details | Formatted Diff | Diff
Save and restore the m_requestingScript flag (7.77 KB, patch)
2006-07-01 07:06 PDT, mitz
no flags Details | Formatted Diff | Diff
Save and restore the m_requestingScript flag (8.04 KB, patch)
2006-07-01 07:09 PDT, mitz
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2006-07-01 03:39:29 PDT
Go to http://digg.com/about and when it's finished loading, click the link to http://digg.com/faq . The latter page (and subsequent pages on digg.com) will be blank or missing some content.

This bug is a regression from r15075 (fix for bug 9317), and it happens when a script loaded from cache writes a <script> tag whose source is also in cache. The requesingScript state bit is messed up as a result. The fix is to replace the state bit with a count.
Comment 1 mitz 2006-07-01 03:53:48 PDT
Created attachment 9118 [details]
Replace the requestingScript state bit with a count
Comment 2 Darin Adler 2006-07-01 06:33:37 PDT
Comment on attachment 9118 [details]
Replace the requestingScript state bit with a count

r=me
Comment 3 Darin Adler 2006-07-01 06:34:45 PDT
Comment on attachment 9118 [details]
Replace the requestingScript state bit with a count

Another fix is to save, set, and then restore the bit rather than setting and clearing it. I think that would be better.

Is there any reason that wouldn't work?
Comment 4 mitz 2006-07-01 06:47:37 PDT
(In reply to comment #3)
> (From update of attachment 9118 [details] [edit])
> Another fix is to save, set, and then restore the bit rather than setting and
> clearing it. I think that would be better.
> 
> Is there any reason that wouldn't work?
> 

I think it would work, but it occurred to me that the bit didn't really belong in State anyway (it is a global property of the tokenizer), and I thought that saving and restoring a bool would just consume byets on the stack with no apparent benefit.

I'm going to attach a save-and-restore version.
Comment 5 mitz 2006-07-01 07:06:30 PDT
Created attachment 9120 [details]
Save and restore the m_requestingScript flag

See my previous comment as to why I think the bit doesn't belong in State. Of course, if you prefer it that way, I can go all the way and put it back into State.
Comment 6 mitz 2006-07-01 07:09:49 PDT
Created attachment 9121 [details]
Save and restore the m_requestingScript flag

Added a missing test subresource. Please see previous comments.
Comment 7 Darin Adler 2006-07-01 14:38:24 PDT
Comment on attachment 9121 [details]
Save and restore the m_requestingScript flag

I think of the m_state as a more-or-less arbitrary set of bits, so the argument about this bit being global state and hence "not part of state" doesn't seem very compelling to me.

Your argument about a byte on the stack vs. a count in the tokenizer object is actually a pretty compelling one. It didn't occur to me that the stack cost can be paid over and over again as you recurse and there is only one tokenizer.

I think that either the original fix or this one is fine. r=me
Comment 8 David Kilzer (:ddkilzer) 2006-07-02 04:34:27 PDT
Committed revision 15134.