Reviewed by Darin Adler.

        <rdar://problem/5665860> With the web inspector displayed, a crash occurs
        at WebCore::Frame::document() when navigating back to previous page

        This fixes the crash, but the inspector was totally broken with back/forward.
        So this also fixes back/forward navigation so the right main resource shows
        up in the inspector.

        * page/InspectorController.cpp:
        (WebCore::addSourceToFrame): Add some null checks for the frame when
        getting the textEncoding. This was the crash.
        (WebCore::InspectorController::addScriptResource): Create a script object
        only if needed, and always add it by calling addResource.
        (WebCore::InspectorController::didCommitLoad): Check if the loader is
        loading from the page cache, and clear m_mainResource. If the load is
        normal, then call addAndUpdateScriptResource with the main resource.
        (WebCore::InspectorController::identifierForInitialRequest): If the load
        is from the page cache and the resource is the main resource call
        addAndUpdateScriptResource since didCommitLoad did not do it.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@29333 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 3f48d1834cf8f6925debb02e3c81ed4d1e2dd3f6..78d4fffa0d8366777c50d313a0c41e4de650a7df 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,26 @@
+2008-01-08  Timothy Hatcher  <timothy@apple.com>
+
+        Reviewed by Darin Adler.
+
+        <rdar://problem/5665860> With the web inspector displayed, a crash occurs
+        at WebCore::Frame::document() when navigating back to previous page
+
+        This fixes the crash, but the inspector was totally broken with back/forward.
+        So this also fixes back/forward navigation so the right main resource shows
+        up in the inspector.
+
+        * page/InspectorController.cpp:
+        (WebCore::addSourceToFrame): Add some null checks for the frame when
+        getting the textEncoding. This was the crash.
+        (WebCore::InspectorController::addScriptResource): Create a script object
+        only if needed, and always add it by calling addResource.
+        (WebCore::InspectorController::didCommitLoad): Check if the loader is 
+        loading from the page cache, and clear m_mainResource. If the load is
+        normal, then call addAndUpdateScriptResource with the main resource. 
+        (WebCore::InspectorController::identifierForInitialRequest): If the load
+        is from the page cache and the resource is the main resource call
+        addAndUpdateScriptResource since didCommitLoad did not do it.
+
 2008-01-08  Alp Toker  <alp@atoker.com>
 
         Back out VIDEO by default in the GTK+ qmake build. The build bot

diff --git a/WebCore/page/InspectorController.cpp b/WebCore/page/InspectorController.cpp
index 37176ea8db659913c9d8bfa24820f541d803e248..bfec4b5e518313aad5c04b27d4bd30300be7003d 100644 (file)
--- a/WebCore/page/InspectorController.cpp
+++ b/WebCore/page/InspectorController.cpp
@@ -141,18 +141,10 @@ struct InspectorResource : public RefCounted<InspectorResource> {
         if (requestURL == loader->requestURL())
             return Doc;
 
-        FrameLoader* frameLoader = loader->frameLoader();
-        if (!frameLoader)
-            return Other;
-
-        if (requestURL == frameLoader->iconURL())
+        if (loader->frameLoader() && requestURL == loader->frameLoader()->iconURL())
             return Image;
 
-        Document* doc = frameLoader->frame()->document();
-        if (!doc)
-            return Other;
-
-        CachedResource* cachedResource = doc->docLoader()->cachedResource(requestURL.string());
+        CachedResource* cachedResource = frame->document()->docLoader()->cachedResource(requestURL.string());
         if (!cachedResource)
             return Other;
 
@@ -274,17 +266,9 @@ static JSValueRef addSourceToFrame(JSContextRef ctx, JSObjectRef /*function*/, J
     String textEncodingName;
     if (resource->requestURL == resource->loader->requestURL()) {
         buffer = resource->loader->mainResourceData();
-        textEncodingName = resource->loader->frame()->document()->inputEncoding();
+        textEncodingName = resource->frame->document()->inputEncoding();
     } else {
-        FrameLoader* frameLoader = resource->loader->frameLoader();
-        if (!frameLoader)
-            return undefined;
-
-        Document* doc = frameLoader->frame()->document();
-        if (!doc)
-            return undefined;
-
-        CachedResource* cachedResource = doc->docLoader()->cachedResource(resource->requestURL.string());
+        CachedResource* cachedResource = resource->frame->document()->docLoader()->cachedResource(resource->requestURL.string());
         if (!cachedResource)
             return undefined;
 
@@ -352,11 +336,7 @@ static JSValueRef getResourceDocumentNode(JSContextRef ctx, JSObjectRef /*functi
     if (!resource)
         return undefined;
 
-    FrameLoader* frameLoader = resource->loader->frameLoader();
-    if (!frameLoader)
-        return undefined;
-
-    Document* document = frameLoader->frame()->document();
+    Document* document = resource->frame->document();
     if (!document)
         return undefined;
 
@@ -928,52 +908,49 @@ JSObjectRef InspectorController::addScriptResource(InspectorResource* resource)
 {
     ASSERT_ARG(resource, resource);
 
-    // This happens for pages loaded from the back/forward cache.
-    if (resource->scriptObject)
-        return resource->scriptObject;
-
     ASSERT(m_scriptContext);
     ASSERT(m_scriptObject);
     if (!m_scriptContext || !m_scriptObject)
         return 0;
 
-    JSRetainPtr<JSStringRef> resourceString(Adopt, JSStringCreateWithUTF8CString("Resource"));
-    JSObjectRef resourceConstructor = JSValueToObject(m_scriptContext, JSObjectGetProperty(m_scriptContext, m_scriptObject, resourceString.get(), 0), 0);
-
-    String urlString = resource->requestURL.string();
-    JSRetainPtr<JSStringRef> url(Adopt, JSStringCreateWithCharacters(urlString.characters(), urlString.length()));
-    JSValueRef urlValue = JSValueMakeString(m_scriptContext, url.get());
+    if (!resource->scriptObject) {
+        JSRetainPtr<JSStringRef> resourceString(Adopt, JSStringCreateWithUTF8CString("Resource"));
+        JSObjectRef resourceConstructor = JSValueToObject(m_scriptContext, JSObjectGetProperty(m_scriptContext, m_scriptObject, resourceString.get(), 0), 0);
 
-    urlString = resource->requestURL.host();
-    JSRetainPtr<JSStringRef> domain(Adopt, JSStringCreateWithCharacters(urlString.characters(), urlString.length()));
-    JSValueRef domainValue = JSValueMakeString(m_scriptContext, domain.get());
+        String urlString = resource->requestURL.string();
+        JSRetainPtr<JSStringRef> url(Adopt, JSStringCreateWithCharacters(urlString.characters(), urlString.length()));
+        JSValueRef urlValue = JSValueMakeString(m_scriptContext, url.get());
 
-    urlString = resource->requestURL.path();
-    JSRetainPtr<JSStringRef> path(Adopt, JSStringCreateWithCharacters(urlString.characters(), urlString.length()));
-    JSValueRef pathValue = JSValueMakeString(m_scriptContext, path.get());
+        urlString = resource->requestURL.host();
+        JSRetainPtr<JSStringRef> domain(Adopt, JSStringCreateWithCharacters(urlString.characters(), urlString.length()));
+        JSValueRef domainValue = JSValueMakeString(m_scriptContext, domain.get());
 
-    urlString = resource->requestURL.lastPathComponent();
-    JSRetainPtr<JSStringRef> lastPathComponent(Adopt, JSStringCreateWithCharacters(urlString.characters(), urlString.length()));
-    JSValueRef lastPathComponentValue = JSValueMakeString(m_scriptContext, lastPathComponent.get());
+        urlString = resource->requestURL.path();
+        JSRetainPtr<JSStringRef> path(Adopt, JSStringCreateWithCharacters(urlString.characters(), urlString.length()));
+        JSValueRef pathValue = JSValueMakeString(m_scriptContext, path.get());
 
-    JSValueRef identifier = JSValueMakeNumber(m_scriptContext, resource->identifier);
-    JSValueRef mainResource = JSValueMakeBoolean(m_scriptContext, m_mainResource == resource);
-    JSValueRef cached = JSValueMakeBoolean(m_scriptContext, resource->cached);
+        urlString = resource->requestURL.lastPathComponent();
+        JSRetainPtr<JSStringRef> lastPathComponent(Adopt, JSStringCreateWithCharacters(urlString.characters(), urlString.length()));
+        JSValueRef lastPathComponentValue = JSValueMakeString(m_scriptContext, lastPathComponent.get());
 
-    JSValueRef arguments[] = { scriptObjectForRequest(m_scriptContext, resource), urlValue, domainValue, pathValue, lastPathComponentValue, identifier, mainResource, cached };
-    JSObjectRef result = JSObjectCallAsConstructor(m_scriptContext, resourceConstructor, 8, arguments, 0);
+        JSValueRef identifier = JSValueMakeNumber(m_scriptContext, resource->identifier);
+        JSValueRef mainResource = JSValueMakeBoolean(m_scriptContext, m_mainResource == resource);
+        JSValueRef cached = JSValueMakeBoolean(m_scriptContext, resource->cached);
 
-    resource->setScriptObject(m_scriptContext, result);
+        JSValueRef arguments[] = { scriptObjectForRequest(m_scriptContext, resource), urlValue, domainValue, pathValue, lastPathComponentValue, identifier, mainResource, cached };
+        JSObjectRef result = JSObjectCallAsConstructor(m_scriptContext, resourceConstructor, 8, arguments, 0);
+        ASSERT(result);
 
-    ASSERT(result);
+        resource->setScriptObject(m_scriptContext, result);
+    }
 
     JSRetainPtr<JSStringRef> addResourceString(Adopt, JSStringCreateWithUTF8CString("addResource"));
     JSObjectRef addResourceFunction = JSValueToObject(m_scriptContext, JSObjectGetProperty(m_scriptContext, m_scriptObject, addResourceString.get(), 0), 0);
 
-    JSValueRef addArguments[] = { result };
+    JSValueRef addArguments[] = { resource->scriptObject };
     JSObjectCallAsFunction(m_scriptContext, addResourceFunction, m_scriptObject, 1, addArguments, 0);
 
-    return result;
+    return resource->scriptObject;
 }
 
 JSObjectRef InspectorController::addAndUpdateScriptResource(InspectorResource* resource)
@@ -1341,9 +1318,6 @@ void InspectorController::didCommitLoad(DocumentLoader* loader)
         return;
 
     if (loader->frame() == m_inspectedPage->mainFrame()) {
-        ASSERT(m_mainResource);
-        // FIXME: Should look into asserting that m_mainResource->loader == loader here.
-
         m_client->inspectedURLChanged(loader->url().string());
 
         deleteAllValues(m_consoleMessages);
@@ -1360,11 +1334,17 @@ void InspectorController::didCommitLoad(DocumentLoader* loader)
 #endif
             clearNetworkTimeline();
 
-            // We don't add the main resource until its load is committed. This
-            // is needed to keep the load for a user-entered URL from showing
-            // up in the list of resources for the page they are navigating
-            // away from.
-            addAndUpdateScriptResource(m_mainResource.get());
+            if (!loader->isLoadingFromCachedPage()) {
+                ASSERT(m_mainResource && m_mainResource->loader == loader);
+                // We don't add the main resource until its load is committed. This is
+                // needed to keep the load for a user-entered URL from showing up in the
+                // list of resources for the page they are navigating away from.
+                addAndUpdateScriptResource(m_mainResource.get());
+            } else {
+                // Pages loaded from the page cache are commited before m_mainResource is the right
+                // resource for this load. Clear it and it will be assigned in identifierForInitialRequest.
+                m_mainResource = 0;
+            }
         }
     }
 
@@ -1453,6 +1433,9 @@ void InspectorController::identifierForInitialRequest(unsigned long identifier,
         m_mainResource = resource;
 
     addResource(resource);
+
+    if (windowVisible() && loader->isLoadingFromCachedPage() && resource == m_mainResource)
+        addAndUpdateScriptResource(resource);
 }
 
 void InspectorController::willSendRequest(DocumentLoader* loader, unsigned long identifier, ResourceRequest& request, const ResourceResponse& redirectResponse)