Closed Bug 457050 Opened 16 years ago Closed 16 years ago

[FIX]Reloading page with javascript: URI asserts in loadgroup code

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

()

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

Attached patch FixSplinter Review
In particular, we end up adding the JSChannel as background, then removing it as not background, and send the mForegroundCount counter negative (or rather very big).

Patch to fix attached.  Not sure how to best test this automatically.
Attachment #340381 - Flags: superreview?(cbiesinger)
Attachment #340381 - Flags: review?(cbiesinger)
Attached patch crashtestSplinter Review
When run using "-reftest", this triggers:

###!!! ASSERTION: ForegroundCount messed up: 'mForegroundCount > 0', file /Users/jruderman/central/netwerk/base/src/nsLoadGroup.cpp, line 679

and a bunch of:

###!!! ASSERTION: Foreground URLs are active.: 'mForegroundCount == 0', file /Users/jruderman/central/netwerk/base/src/nsLoadGroup.cpp, line 355
Sweet.  Those are exactly what I was seeing.
Keywords: assertion, testcase
Comment on attachment 340381 [details] [diff] [review]
Fix

+        PRBool loadGroupIsBackround = PR_FALSE;

missing g in the variable name

but... why isn't it enough to check mActualLoadFlags & LOAD_BACKGROUND? why do you have to check the load group?
Because the loadgroup as a whole may be LOAD_BACKGROUND, and that flag might be getting set on us via MergeLoadGlags.  That's where the SetLoadFlags call that confuses us comes from in the testcase.
Attachment #340381 - Flags: superreview?(cbiesinger)
Attachment #340381 - Flags: superreview+
Attachment #340381 - Flags: review?(cbiesinger)
Attachment #340381 - Flags: review+
Pushed changeset dd1c08d6d993.
Flags: in-testsuite+
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
QA Contact: content
Component: Content → DOM
QA Contact: content → general
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: