Reviewed by Geoff.
authorthatcher <thatcher@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Jul 2007 23:44:17 +0000 (23:44 +0000)
committerthatcher <thatcher@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Jul 2007 23:44:17 +0000 (23:44 +0000)
        <rdar://problem/5326126> InspectorResource holds onto more data than it needs

        Reduces the fixed size of InspectorResource from 630 bytes to 224 bytes. Now
        selectively pick out parts of the ResourceRequest and ResourceResponse objects
        for the info needed and store that directly in InspectorResource.

        The ResourceRequest and ResourceResponse objects hold a reference to the original
        NSURL object, so almost double the data was being kept around. So the struct size
        reduction is just the tip of the ice burg on what this really saves.

        In a test of 100,000 XMLHTTPRequests using a 85 byte long data URL, I see ~21 MB less
        RSIZE compared to Safari running without this change.

        * page/InspectorController.cpp:
        (WebCore::InspectorResource::InspectorResource):
        (WebCore::InspectorResource::type):
        (WebCore::addSourceToFrame):
        (WebCore::scriptObjectForRequest):
        (WebCore::scriptObjectForResponse):
        (WebCore::InspectorController::addScriptResource):
        (WebCore::InspectorController::addAndUpdateScriptResource):
        (WebCore::updateResourceRequest):
        (WebCore::updateResourceResponse):
        (WebCore::InspectorController::updateScriptResourceRequest):
        (WebCore::InspectorController::updateScriptResourceResponse):
        (WebCore::InspectorController::didLoadResourceFromMemoryCache):
        (WebCore::InspectorController::identifierForInitialRequest):
        (WebCore::InspectorController::willSendRequest):
        (WebCore::InspectorController::didReceiveResponse):
        (WebCore::InspectorController::didFailLoading):
        * page/InspectorController.h:

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

WebCore/ChangeLog
WebCore/WebCore.xcodeproj/project.pbxproj
WebCore/page/InspectorController.cpp
WebCore/page/InspectorController.h

index 811fb57..376ca90 100644 (file)
@@ -1,3 +1,39 @@
+2007-07-10  Timothy Hatcher  <timothy@apple.com>
+
+        Reviewed by Geoff.
+
+        <rdar://problem/5326126> InspectorResource holds onto more data than it needs
+
+        Reduces the fixed size of InspectorResource from 630 bytes to 224 bytes. Now
+        selectively pick out parts of the ResourceRequest and ResourceResponse objects
+        for the info needed and store that directly in InspectorResource.
+
+        The ResourceRequest and ResourceResponse objects hold a reference to the original
+        NSURL object, so almost double the data was being kept around. So the struct size
+        reduction is just the tip of the ice burg on what this really saves.
+
+        In a test of 100,000 XMLHTTPRequests using a 85 byte long data URL, I see ~21 MB less
+        RSIZE compared to Safari running without this change.
+
+        * page/InspectorController.cpp:
+        (WebCore::InspectorResource::InspectorResource):
+        (WebCore::InspectorResource::type):
+        (WebCore::addSourceToFrame):
+        (WebCore::scriptObjectForRequest):
+        (WebCore::scriptObjectForResponse):
+        (WebCore::InspectorController::addScriptResource):
+        (WebCore::InspectorController::addAndUpdateScriptResource):
+        (WebCore::updateResourceRequest):
+        (WebCore::updateResourceResponse):
+        (WebCore::InspectorController::updateScriptResourceRequest):
+        (WebCore::InspectorController::updateScriptResourceResponse):
+        (WebCore::InspectorController::didLoadResourceFromMemoryCache):
+        (WebCore::InspectorController::identifierForInitialRequest):
+        (WebCore::InspectorController::willSendRequest):
+        (WebCore::InspectorController::didReceiveResponse):
+        (WebCore::InspectorController::didFailLoading):
+        * page/InspectorController.h:
+
 2007-07-10  Darin Adler  <darin@apple.com>
 
         Reviewed by Brady.
index 81be01f..35d0fa6 100644 (file)
                0867D690FE84028FC02AAC07 /* Project object */ = {
                        isa = PBXProject;
                        buildConfigurationList = 149C284308902B11008A9EFC /* Build configuration list for PBXProject "WebCore" */;
+                       compatibilityVersion = "Xcode 2.4";
                        hasScannedForEncodings = 1;
                        knownRegions = (
                                English,
                        mainGroup = 0867D691FE84028FC02AAC07 /* WebKit */;
                        productRefGroup = 034768DFFF38A50411DB9C8B /* Products */;
                        projectDirPath = "";
+                       projectRoot = "";
                        targets = (
                                93F198A508245E59001E9ABC /* WebCore */,
                                DD041FBE09D9DDBE0010AF2A /* Derived Sources */,
index a5dbba3..cb0dc5f 100644 (file)
@@ -44,7 +44,6 @@
 #include "JSRange.h"
 #include "Page.h"
 #include "Range.h"
-#include "ResourceError.h"
 #include "ResourceRequest.h"
 #include "ResourceResponse.h"
 #include "Shared.h"
@@ -94,10 +93,12 @@ struct InspectorResource : public Shared<InspectorResource> {
         , frame(frame)
         , scriptContext(0)
         , scriptObject(0)
+        , expectedContentLength(0)
         , cached(false)
         , finished(false)
         , failed(false)
         , length(0)
+        , responseStatusCode(0)
         , startTime(-1.0)
         , responseReceivedTime(-1.0)
         , endTime(-1.0)
@@ -111,24 +112,24 @@ struct InspectorResource : public Shared<InspectorResource> {
 
     Type type() const
     {
-        if (request.isNull() || response.isNull())
+        if (!requestURL.isValid())
             return Other;
 
-        if (request.url() == loader->requestURL())
+        if (requestURL == loader->requestURL())
             return Doc;
 
         FrameLoader* frameLoader = loader->frameLoader();
         if (!frameLoader)
             return Other;
 
-        if (request.url() == frameLoader->iconURL())
+        if (requestURL == frameLoader->iconURL())
             return Image;
 
         Document* doc = frameLoader->frame()->document();
         if (!doc)
             return Other;
 
-        CachedResource* cachedResource = doc->docLoader()->cachedResource(request.url().url());
+        CachedResource* cachedResource = doc->docLoader()->cachedResource(requestURL.url());
         if (!cachedResource)
             return Other;
 
@@ -163,16 +164,20 @@ struct InspectorResource : public Shared<InspectorResource> {
     long long identifier;
     RefPtr<DocumentLoader> loader;
     RefPtr<Frame> frame;
-    ResourceRequest request;
-    ResourceResponse response;
-    ResourceResponse redirectResponse;
-    ResourceError error;
+    KURL requestURL;
+    HTTPHeaderMap requestHeaderFields;
+    HTTPHeaderMap responseHeaderFields;
+    String mimeType;
+    String suggestedFilename;
+    String textEncodingName;
     JSContextRef scriptContext;
     JSObjectRef scriptObject;
+    long long expectedContentLength;
     bool cached;
     bool finished;
     bool failed;
     int length;
+    int responseStatusCode;
     double startTime;
     double responseReceivedTime;
     double endTime;
@@ -197,7 +202,7 @@ static JSValueRef addSourceToFrame(JSContextRef ctx, JSObjectRef /*function*/, J
         return undefined;
 
     RefPtr<SharedBuffer> buffer;
-    if (resource->request.url() == resource->loader->requestURL())
+    if (resource->requestURL == resource->loader->requestURL())
         buffer = resource->loader->mainResourceData();
     else {
         FrameLoader* frameLoader = resource->loader->frameLoader();
@@ -208,7 +213,7 @@ static JSValueRef addSourceToFrame(JSContextRef ctx, JSObjectRef /*function*/, J
         if (!doc)
             return undefined;
 
-        CachedResource* cachedResource = doc->docLoader()->cachedResource(resource->request.url().url());
+        CachedResource* cachedResource = doc->docLoader()->cachedResource(resource->requestURL.url());
         if (!cachedResource)
             return undefined;
 
@@ -220,7 +225,7 @@ static JSValueRef addSourceToFrame(JSContextRef ctx, JSObjectRef /*function*/, J
 
     String textEncodingName = resource->loader->overrideEncoding();
     if (!textEncodingName)
-        textEncodingName = resource->response.textEncodingName();
+        textEncodingName = resource->textEncodingName;
 
     TextEncoding encoding(textEncodingName);
     if (!encoding.isValid())
@@ -253,7 +258,7 @@ static JSValueRef addSourceToFrame(JSContextRef ctx, JSObjectRef /*function*/, J
 
     FrameLoader* loader = frameOwner->contentFrame()->loader();
 
-    loader->setResponseMIMEType(resource->response.mimeType());
+    loader->setResponseMIMEType(resource->mimeType);
     loader->begin();
     loader->write(sourceString);
     loader->end();
@@ -679,23 +684,23 @@ static void addHeaders(JSContextRef context, JSObjectRef object, const HTTPHeade
     }
 }
 
-static JSObjectRef objectForRequest(JSContextRef context, const ResourceRequest& request)
+static JSObjectRef scriptObjectForRequest(JSContextRef context, const InspectorResource* resource)
 {
     ASSERT_ARG(context, context);
-    
+
     JSObjectRef object = JSObjectMake(context, 0, 0);
-    addHeaders(context, object, request.httpHeaderFields());
-    
+    addHeaders(context, object, resource->requestHeaderFields);
+
     return object;
 }
 
-static JSObjectRef objectForResponse(JSContextRef context, const ResourceResponse& response)
+static JSObjectRef scriptObjectForResponse(JSContextRef context, const InspectorResource* resource)
 {
     ASSERT_ARG(context, context);
-    
+
     JSObjectRef object = JSObjectMake(context, 0, 0);
-    addHeaders(context, object, response.httpHeaderFields());
-    
+    addHeaders(context, object, resource->responseHeaderFields);
+
     return object;
 }
 
@@ -713,22 +718,22 @@ JSObjectRef InspectorController::addScriptResource(InspectorResource* resource)
     JSObjectRef resourceConstructor = JSValueToObject(m_scriptContext, JSObjectGetProperty(m_scriptContext, m_scriptObject, resourceString, 0), 0);
     JSStringRelease(resourceString);
 
-    String urlString = resource->request.url().url();
+    String urlString = resource->requestURL.url();
     JSStringRef url = JSStringCreateWithCharacters(urlString.characters(), urlString.length());
     JSValueRef urlValue = JSValueMakeString(m_scriptContext, url);
     JSStringRelease(url);
 
-    urlString = resource->request.url().host();
+    urlString = resource->requestURL.host();
     JSStringRef domain = JSStringCreateWithCharacters(urlString.characters(), urlString.length());
     JSValueRef domainValue = JSValueMakeString(m_scriptContext, domain);
     JSStringRelease(domain);
 
-    urlString = resource->request.url().path();
+    urlString = resource->requestURL.path();
     JSStringRef path = JSStringCreateWithCharacters(urlString.characters(), urlString.length());
     JSValueRef pathValue = JSValueMakeString(m_scriptContext, path);
     JSStringRelease(path);
 
-    urlString = resource->request.url().lastPathComponent();
+    urlString = resource->requestURL.lastPathComponent();
     JSStringRef lastPathComponent = JSStringCreateWithCharacters(urlString.characters(), urlString.length());
     JSValueRef lastPathComponentValue = JSValueMakeString(m_scriptContext, lastPathComponent);
     JSStringRelease(lastPathComponent);
@@ -737,7 +742,7 @@ JSObjectRef InspectorController::addScriptResource(InspectorResource* resource)
     JSValueRef mainResource = JSValueMakeBoolean(m_scriptContext, m_mainResource == resource);
     JSValueRef cached = JSValueMakeBoolean(m_scriptContext, resource->cached);
 
-    JSValueRef arguments[] = { objectForRequest(m_scriptContext, resource->request), urlValue, domainValue, pathValue, lastPathComponentValue, identifier, mainResource, cached };
+    JSValueRef arguments[] = { scriptObjectForRequest(m_scriptContext, resource), urlValue, domainValue, pathValue, lastPathComponentValue, identifier, mainResource, cached };
     JSObjectRef result = JSObjectCallAsConstructor(m_scriptContext, resourceConstructor, 8, arguments, 0);
 
     resource->setScriptObject(m_scriptContext, result);
@@ -759,7 +764,7 @@ JSObjectRef InspectorController::addAndUpdateScriptResource(InspectorResource* r
     ASSERT_ARG(resource, resource);
 
     JSObjectRef scriptResource = addScriptResource(resource);
-    updateScriptResource(resource, resource->response);
+    updateScriptResourceResponse(resource);
     updateScriptResource(resource, resource->length);
     updateScriptResource(resource, resource->startTime, resource->responseReceivedTime, resource->endTime);
     updateScriptResource(resource, resource->finished, resource->failed);
@@ -788,29 +793,45 @@ void InspectorController::removeScriptResource(InspectorResource* resource)
     resource->setScriptObject(0, 0);
 }
 
-void InspectorController::updateScriptResource(InspectorResource* resource, const ResourceRequest& request)
+static void updateResourceRequest(InspectorResource* resource, const ResourceRequest& request)
+{
+    resource->requestHeaderFields = request.httpHeaderFields();
+    resource->requestURL = request.url();
+}
+
+static void updateResourceResponse(InspectorResource* resource, const ResourceResponse& response)
+{
+    resource->expectedContentLength = response.expectedContentLength();
+    resource->mimeType = response.mimeType();
+    resource->responseHeaderFields = response.httpHeaderFields();
+    resource->responseStatusCode = response.httpStatusCode();
+    resource->suggestedFilename = response.suggestedFilename();
+    resource->textEncodingName = response.textEncodingName();
+}
+
+void InspectorController::updateScriptResourceRequest(InspectorResource* resource)
 {
     ASSERT(resource->scriptObject);
     ASSERT(m_scriptContext);
     if (!resource->scriptObject || !m_scriptContext)
         return;
 
-    String urlString = request.url().url();
+    String urlString = resource->requestURL.url();
     JSStringRef url = JSStringCreateWithCharacters(urlString.characters(), urlString.length());
     JSValueRef urlValue = JSValueMakeString(m_scriptContext, url);
     JSStringRelease(url);
 
-    urlString = request.url().host();
+    urlString = resource->requestURL.host();
     JSStringRef domain = JSStringCreateWithCharacters(urlString.characters(), urlString.length());
     JSValueRef domainValue = JSValueMakeString(m_scriptContext, domain);
     JSStringRelease(domain);
 
-    urlString = request.url().path();
+    urlString = resource->requestURL.path();
     JSStringRef path = JSStringCreateWithCharacters(urlString.characters(), urlString.length());
     JSValueRef pathValue = JSValueMakeString(m_scriptContext, path);
     JSStringRelease(path);
 
-    urlString = request.url().lastPathComponent();
+    urlString = resource->requestURL.lastPathComponent();
     JSStringRef lastPathComponent = JSStringCreateWithCharacters(urlString.characters(), urlString.length());
     JSValueRef lastPathComponentValue = JSValueMakeString(m_scriptContext, lastPathComponent);
     JSStringRelease(lastPathComponent);
@@ -834,7 +855,7 @@ void InspectorController::updateScriptResource(InspectorResource* resource, cons
     JSStringRelease(propertyName);
 
     propertyName = JSStringCreateWithUTF8CString("requestHeaders");
-    JSObjectSetProperty(m_scriptContext, resource->scriptObject, propertyName, objectForRequest(m_scriptContext, request), kJSPropertyAttributeNone, 0);
+    JSObjectSetProperty(m_scriptContext, resource->scriptObject, propertyName, scriptObjectForRequest(m_scriptContext, resource), kJSPropertyAttributeNone, 0);
     JSStringRelease(propertyName);
 
     propertyName = JSStringCreateWithUTF8CString("mainResource");
@@ -842,23 +863,23 @@ void InspectorController::updateScriptResource(InspectorResource* resource, cons
     JSStringRelease(propertyName);
 }
 
-void InspectorController::updateScriptResource(InspectorResource* resource, const ResourceResponse& response, bool /*redirect*/)
+void InspectorController::updateScriptResourceResponse(InspectorResource* resource)
 {
     ASSERT(resource->scriptObject);
     ASSERT(m_scriptContext);
     if (!resource->scriptObject || !m_scriptContext)
         return;
 
-    JSStringRef mimeType = JSStringCreateWithCharacters(response.mimeType().characters(), response.mimeType().length());
+    JSStringRef mimeType = JSStringCreateWithCharacters(resource->mimeType.characters(), resource->mimeType.length());
     JSValueRef mimeTypeValue = JSValueMakeString(m_scriptContext, mimeType);
     JSStringRelease(mimeType);
 
-    JSStringRef suggestedFilename = JSStringCreateWithCharacters(response.suggestedFilename().characters(), response.suggestedFilename().length());
+    JSStringRef suggestedFilename = JSStringCreateWithCharacters(resource->suggestedFilename.characters(), resource->suggestedFilename.length());
     JSValueRef suggestedFilenameValue = JSValueMakeString(m_scriptContext, suggestedFilename);
     JSStringRelease(suggestedFilename);
 
-    JSValueRef expectedContentLengthValue = JSValueMakeNumber(m_scriptContext, static_cast<double>(response.expectedContentLength()));
-    JSValueRef statusCodeValue = JSValueMakeNumber(m_scriptContext, response.httpStatusCode());
+    JSValueRef expectedContentLengthValue = JSValueMakeNumber(m_scriptContext, static_cast<double>(resource->expectedContentLength));
+    JSValueRef statusCodeValue = JSValueMakeNumber(m_scriptContext, resource->responseStatusCode);
 
     JSStringRef propertyName = JSStringCreateWithUTF8CString("mimeType");
     JSObjectSetProperty(m_scriptContext, resource->scriptObject, propertyName, mimeTypeValue, kJSPropertyAttributeNone, 0);
@@ -877,7 +898,7 @@ void InspectorController::updateScriptResource(InspectorResource* resource, cons
     JSStringRelease(propertyName);
     
     propertyName = JSStringCreateWithUTF8CString("responseHeaders");
-    JSObjectSetProperty(m_scriptContext, resource->scriptObject, propertyName, objectForResponse(m_scriptContext, response), kJSPropertyAttributeNone, 0);
+    JSObjectSetProperty(m_scriptContext, resource->scriptObject, propertyName, scriptObjectForResponse(m_scriptContext, resource), kJSPropertyAttributeNone, 0);
     JSStringRelease(propertyName);
 
     JSValueRef typeValue = JSValueMakeNumber(m_scriptContext, resource->type());
@@ -886,11 +907,6 @@ void InspectorController::updateScriptResource(InspectorResource* resource, cons
     JSStringRelease(propertyName);
 }
 
-void InspectorController::updateScriptResource(InspectorResource*, const ResourceError&)
-{
-    // FIXME: Do something here when we start showing HTTP errors in the Inspector.
-}
-
 void InspectorController::updateScriptResource(InspectorResource* resource, int length)
 {
     ASSERT(resource->scriptObject);
@@ -1126,8 +1142,10 @@ void InspectorController::didLoadResourceFromMemoryCache(DocumentLoader* loader,
 {
     InspectorResource* resource = new InspectorResource(m_nextIdentifier--, loader, loader->frame());
     resource->finished = true;
-    resource->request = request;
-    resource->response = response;
+
+    updateResourceRequest(resource, request);
+    updateResourceResponse(resource, response);
+
     resource->length = length;
     resource->cached = true;
     resource->startTime = currentTime();
@@ -1146,7 +1164,8 @@ void InspectorController::didLoadResourceFromMemoryCache(DocumentLoader* loader,
 void InspectorController::identifierForInitialRequest(unsigned long identifier, DocumentLoader* loader, const ResourceRequest& request)
 {
     InspectorResource* resource = new InspectorResource(identifier, loader, loader->frame());
-    resource->request = request;
+
+    updateResourceRequest(resource, request);
 
     if (loader->frame() == m_inspectedPage->mainFrame() && request.url() == loader->requestURL())
         m_mainResource = resource;
@@ -1162,22 +1181,21 @@ void InspectorController::willSendRequest(DocumentLoader* loader, unsigned long
 
     resource->startTime = currentTime();
 
-    if (resource->request != request)
-        resource->request = request;
-
-    if (!redirectResponse.isNull())
-        resource->redirectResponse = redirectResponse;
+    if (!redirectResponse.isNull()) {
+        updateResourceRequest(resource, request);
+        updateResourceResponse(resource, redirectResponse);
+    }
 
     if (resource != m_mainResource && windowVisible()) {
         if (!resource->scriptObject)
             addScriptResource(resource);
         else
-            updateScriptResource(resource, resource->request);
+            updateScriptResourceRequest(resource);
 
         updateScriptResource(resource, resource->startTime, resource->responseReceivedTime, resource->endTime);
 
         if (!redirectResponse.isNull())
-            updateScriptResource(resource, redirectResponse, true);
+            updateScriptResourceResponse(resource);
     }
 }
 
@@ -1187,11 +1205,12 @@ void InspectorController::didReceiveResponse(DocumentLoader*, unsigned long iden
     if (!resource)
         return;
 
-    resource->response = response;
+    updateResourceResponse(resource, response);
+
     resource->responseReceivedTime = currentTime();
 
     if (windowVisible() && resource->scriptObject) {
-        updateScriptResource(resource, response);
+        updateScriptResourceResponse(resource);
         updateScriptResource(resource, resource->startTime, resource->responseReceivedTime, resource->endTime);
     }
 }
@@ -1227,7 +1246,7 @@ void InspectorController::didFinishLoading(DocumentLoader* loader, unsigned long
     }
 }
 
-void InspectorController::didFailLoading(DocumentLoader* loader, unsigned long identifier, const ResourceError& error)
+void InspectorController::didFailLoading(DocumentLoader* loader, unsigned long identifier, const ResourceError& /*error*/)
 {
     RefPtr<InspectorResource> resource = m_resources.get(identifier);
     if (!resource)
@@ -1237,13 +1256,11 @@ void InspectorController::didFailLoading(DocumentLoader* loader, unsigned long i
 
     resource->finished = true;
     resource->failed = true;
-    resource->error = error;
     resource->endTime = currentTime();
 
     addResource(resource.get());
 
     if (windowVisible() && resource->scriptObject) {
-        updateScriptResource(resource.get(), resource->error);
         updateScriptResource(resource.get(), resource->startTime, resource->responseReceivedTime, resource->endTime);
         updateScriptResource(resource.get(), resource->finished, resource->failed);
     }
index 33b7d2f..7035467 100644 (file)
@@ -110,9 +110,8 @@ private:
     void removeScriptResource(InspectorResource*);
 
     JSObjectRef addAndUpdateScriptResource(InspectorResource*);
-    void updateScriptResource(InspectorResource*, const ResourceRequest&);
-    void updateScriptResource(InspectorResource*, const ResourceResponse&, bool redirect = false);
-    void updateScriptResource(InspectorResource*, const ResourceError&);
+    void updateScriptResourceRequest(InspectorResource*);
+    void updateScriptResourceResponse(InspectorResource*);
     void updateScriptResource(InspectorResource*, int length);
     void updateScriptResource(InspectorResource*, bool finished, bool failed = false);
     void updateScriptResource(InspectorResource*, double startTime, double responseReceivedTime, double endTime);