Bug 24067 - REGRESSION: Crash in WebCore::Document::initSecurityContext
Summary: REGRESSION: Crash in WebCore::Document::initSecurityContext
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2009-02-20 15:00 PST by Holger Freyther
Modified: 2009-02-25 09:01 PST (History)
2 users (show)

See Also:


Attachments
test case (crash) (409 bytes, text/html)
2009-02-21 05:17 PST, Alexey Proskuryakov
no flags Details
proposed fix (5.28 KB, patch)
2009-02-24 05:25 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Holger Freyther 2009-02-20 15:00:36 PST
A slightly modified test of fast/dom/Window/window-open-self.html is crashing. I'm 99% confident that this regression was introduced by r40824. This is crashing mac, Qt and Gtk.
Comment 1 Holger Freyther 2009-02-20 15:02:51 PST
This was uncovered because the Gtk+ DumpRenderTree is loading about:blank after each test. It might be more easy to apply this by hand than applying a patch.

diff --git a/LayoutTests/fast/dom/Window/resources/destination.html b/LayoutTests/fast/dom/Window/resources/destination.html
index 33d0269..4e9064e 100644
--- a/LayoutTests/fast/dom/Window/resources/destination.html
+++ b/LayoutTests/fast/dom/Window/resources/destination.html
@@ -1,2 +1,2 @@
 <p>Hooray, you got here! That means the test succeeded!</p>
-<script>if (window.layoutTestController) layoutTestController.notifyDone();</script>
+<script>window.open("about:blank", "_self");</script>
Comment 2 Holger Freyther 2009-02-20 15:03:39 PST
Backtrace (inline to ease searching):

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xb577a700 (LWP 1335)]
0xb76e0336 in WebCore::Document::initSecurityContext () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
Current language:  auto; currently c++
(gdb) bt
#0  0xb76e0336 in WebCore::Document::initSecurityContext () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#1  0xb76e2d56 in WebCore::Document::Document () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#2  0xb77c48b4 in WebCore::HTMLDocument::HTMLDocument () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#3  0xb76d756f in WebCore::DOMImplementation::createDocument () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#4  0xb785556d in WebCore::FrameLoader::begin () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#5  0xb785cebc in WebCore::FrameLoader::receivedFirstData () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#6  0xb785d29e in WebCore::FrameLoader::setEncoding () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#7  0xb75f9b25 in WebKit::FrameLoaderClient::committedLoad () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#8  0xb75f7cc2 in WebKit::FrameLoaderClient::finishedLoading () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#9  0xb785f558 in WebCore::FrameLoader::finishedLoadingDocument () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#10 0xb783d6d6 in WebCore::DocumentLoader::finishedLoading () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#11 0xb7852693 in WebCore::FrameLoader::finishedLoading () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#12 0xb7864a50 in WebCore::MainResourceLoader::didFinishLoading () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#13 0xb7866dd6 in WebCore::MainResourceLoader::continueAfterContentPolicy () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#14 0xb786705e in WebCore::MainResourceLoader::continueAfterContentPolicy () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#15 0xb7852d45 in WebCore::FrameLoader::continueAfterContentPolicy () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#16 0xb7602a6e in webkit_web_policy_decision_use () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#17 0xb75fa990 in WebKit::FrameLoaderClient::dispatchDecidePolicyForMIMEType () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#18 0xb785198c in WebCore::FrameLoader::checkContentPolicy () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#19 0xb78658c7 in WebCore::MainResourceLoader::didReceiveResponse () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#20 0xb7864eeb in WebCore::MainResourceLoader::handleEmptyLoad () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#21 0xb7866a13 in WebCore::MainResourceLoader::loadNow () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#22 0xb7867f6e in WebCore::MainResourceLoader::load () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#23 0xb783e56e in WebCore::DocumentLoader::startLoadingMainResource () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#24 0xb784ba04 in WebCore::FrameLoader::continueLoadAfterWillSubmitForm () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#25 0xb78529e8 in WebCore::FrameLoader::continueLoadAfterNavigationPolicy () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#26 0xb7852a25 in WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#27 0xb784f118 in WebCore::PolicyCheck::call () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#28 0xb7852b8e in WebCore::FrameLoader::continueAfterNavigationPolicy () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#29 0xb7602a6e in webkit_web_policy_decision_use () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#30 0xb75fad1e in WebKit::FrameLoaderClient::dispatchDecidePolicyForNavigationAction () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#31 0xb78507bf in WebCore::FrameLoader::checkNavigationPolicy () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#32 0xb7851ed4 in WebCore::FrameLoader::loadWithDocumentLoader () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#33 0xb7856737 in WebCore::FrameLoader::loadWithNavigationAction () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#34 0xb7856b7d in WebCore::FrameLoader::loadURL () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#35 0xb7856cd8 in WebCore::FrameLoader::loadURL () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#36 0xb785930b in WebCore::FrameLoader::loadFrameRequestWithFormAndValues () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#37 0xb785a4a0 in WebCore::FrameLoader::urlSelected () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#38 0xb785a5ef in WebCore::FrameLoader::urlSelected () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#39 0xb785a8f0 in WebCore::FrameLoader::changeLocation () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#40 0xb785a9d5 in WebCore::FrameLoader::changeLocation () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#41 0xb785aa90 in WebCore::FrameLoader::redirectionTimerFired () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#42 0xb78608d1 in WebCore::Timer<WebCore::FrameLoader>::fired () from /home/ich/source/apple/WebKit-tt.git/WebKitBuild/Release/.libs/libwebkit-1.0.so.1
#43 0xb78f2caa in WebCore::ThreadTimers::fireTimers (this=0xb568b410, fireTime=1235170297.4847701, firingTimers=@0xbf88a39c) at ../../WebCore/platform/ThreadTimers.cpp:111
#44 0xb78f2d57 in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0xb568b410) at ../../WebCore/platform/ThreadTimers.cpp:141
#45 0xb78f2de2 in WebCore::ThreadTimers::sharedTimerFired () at ../../WebCore/platform/ThreadTimers.cpp:122
Comment 3 Alexey Proskuryakov 2009-02-21 05:17:56 PST
Created attachment 27852 [details]
test case (crash)

There are two ways to fix this bug:
1) Restore the null check in Document::initSecurityContext().
2) Change FrameLoader::begin() to create a new document before detaching the old one.

The former will just restore the old behavior, but the latter might be more correct, because the below code from initSecurityContext would run. I don't know how to test for security context aliasing to find out what Firefox does in this particular case.

    if (ownerFrame) {
        m_cookieURL = ownerFrame->document()->cookieURL();
        // We alias the SecurityOrigins to match Firefox, see Bug 15313
        // https://bugs.webkit.org/show_bug.cgi?id=15313
        ScriptExecutionContext::setSecurityOrigin(ownerFrame->document()->securityOrigin());
    }
Comment 4 Adam Barth 2009-02-22 23:02:22 PST
(In reply to comment #3)
> I don't know how to test for security context aliasing
> to find out what Firefox does in this particular case.

You can test for the aliasing by setting document.domain in one frame and seeing if the document.domain value changes in the other frame.
Comment 5 Alexey Proskuryakov 2009-02-23 13:50:36 PST
> You can test for the aliasing by setting document.domain in one frame and
> seeing if the document.domain value changes in the other frame.

I guess it doesn't matter then, as it's the same frame (opening _self just replaces content).
Comment 6 Adam Barth 2009-02-23 13:53:59 PST
(In reply to comment #5)
> I guess it doesn't matter then, as it's the same frame (opening _self just
> replaces content).

It matters if the first frame has a security context that is already aliased to another frame.  :)
Comment 7 Alexey Proskuryakov 2009-02-24 03:43:31 PST
Tricky! Firefox inherits aliasing in this case, but WebKit did not. So, removing this null check helped to uncover a bug.

Running tests now to check that moving document construction doesn't break anything else.
Comment 8 Alexey Proskuryakov 2009-02-24 05:25:27 PST
Created attachment 27915 [details]
proposed fix
Comment 9 Darin Adler 2009-02-24 09:37:05 PST
Comment on attachment 27915 [details]
proposed fix

r=me
Comment 10 Holger Freyther 2009-02-24 10:56:04 PST
wow!
Comment 11 Alexey Proskuryakov 2009-02-25 09:01:39 PST
Committed revision 41213.