Bug 21752

Summary:

REGRESSION: referencing XHR constructor for a not yet loaded frame permanently breaks it

Product:

WebKit

Reporter:

Alexey Proskuryakov <ap>

Component:

WebCore JavaScript

Assignee:

Alexey Proskuryakov <ap>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, sam

Priority:

Keywords:

HasReduction, InRadar, Regression

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

OS X 10.5

Attachments:

Description	Flags
test case	none
proposed fix	darin: review+

Alexey Proskuryakov

Reported 2008-10-20 07:47:51 PDT

To reproduce, see attached test case. This is a regression caused by r36954, which enabled certain constructors to be cached on the Window, as they should be. Unfortunately, this conflicts with a pre-existing issue in WebCore - the Window object is not properly replaced/cleared when transitioning from a temporary document created before loading to an actual one. The attached test case checks both the pre-existing issue, and its new manifestation.

Attachments
test case (1.55 KB, application/zip) 2008-10-20 07:48 PDT, Alexey Proskuryakov	no flags	Details
proposed fix (17.21 KB, patch) 2009-03-02 07:40 PST, Alexey Proskuryakov	darin: review+	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Alexey Proskuryakov

Comment 1 2008-10-20 07:48:35 PDT

Created attachment 24523 [details] test case

Alexey Proskuryakov

Comment 2 2008-10-20 07:50:06 PDT

<rdar://problem/6303355>

Alexey Proskuryakov

Comment 3 2009-03-02 06:06:33 PST

In fact, this "pre-existing issue" is expected behavior, see <http://trac.webkit.org/changeset/25783/trunk/WebCore/loader/FrameLoader.cpp>. The attached test is faulty, as the way logging is done affects its results. The problem with XMLHttpRequest being broken remains.

Alexey Proskuryakov

Comment 4 2009-03-02 07:40:13 PST

Created attachment 28175 [details] proposed fix

Adam Barth

Comment 5 2009-03-02 09:30:47 PST

Have you tested whether this creates memory leaks? It seems like the JSGlobalObject holds on to the JSImageConstructor, which now mark()s the JSGlobalObject...

Adam Barth

Comment 6 2009-03-02 09:43:51 PST

Ignore my previous comment. I forgot that we use mark-and-sweep.

Sam Weinig

Comment 7 2009-03-13 12:27:12 PDT

In what cases are we replacing the document but not the window object. Ideally, these should be one-to-one now.

Darin Adler

Comment 8 2009-03-13 12:32:44 PDT

Comment on attachment 28175 [details] proposed fix r=me I'm slightly worried since it would be insecure to re-use these JavaScript objects with a new document with a different security origin. But I guess we have tests to cover that possibility and they are not re-used in that bad way.

Alexey Proskuryakov

Comment 9 2009-03-13 12:38:44 PDT

Sam, one case where we must replace only the document is in r25783.

Alexey Proskuryakov

Comment 10 2009-03-16 00:40:55 PDT

Committed <http://trac.webkit.org/changeset/41732>. > I'm slightly worried since it would be insecure to re-use these JavaScript > objects with a new document with a different security origin. My understanding is that this patch doesn't make security weaker, because the only transition that ever happens is from about:blank.

Note You need to log in before you can comment on or make changes to this bug.