Reviewed by Brady.
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Nov 2006 20:41:03 +0000 (20:41 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Nov 2006 20:41:03 +0000 (20:41 +0000)
        - fix <rdar://problem/4752069> 9A274: World of Warcraft Launcher
          crashes on launch in WebCore::ResourceLoader::start

        No layout test, because this depends on cached icons, although there's
        perhaps a way to write a test for it with some further ingenuity.

        * loader/icon/IconLoader.h: Make IconLoader inherit from Noncopyable
        to make explicit the fact that it can't be successfully copied.
        Remove notifyIconChanged function and put the contents in the caller.
        This eliminates the need for IconLoaderMac.mm. Added finishedLoading
        and clearLoadingState functions to share code. Removed m_url, since the
        resource handle already stores the URL. Renamed m_resourceLoader to
        m_handle to reflect the class's name change. Removed the 4096-byte
        inline buffer from m_data, since the malloc savings is not sufficient
        to offset the additional memory use. Removed m_httpStatusCode because
        we can instead cancel the load when we get a status code that reflects
        failure. Added m_loadIsInProgress boolean because we need to detect
        loads that complete during the ResourceHandle::create function call.

        * loader/icon/IconLoader.cpp:
        (WebCore::IconLoader::IconLoader): Initialize m_loadIsInProgress.
        Don't initialize m_httpStatusCode.
        (WebCore::IconLoader::~IconLoader): Updated for name change.
        (WebCore::IconLoader::startLoading): Added code to use the
        m_loadIsInProgress flag to detect if the load completed while inside
        the ResourceHandle::create function. Removed code to set m_url.
        (WebCore::IconLoader::stopLoading): Call clearLoadingState to share
        more code.
        (WebCore::IconLoader::didReceiveResponse): Kill the ResourceHandle
        and finish loading if the HTTP status code indicates failure.
        (WebCore::IconLoader::didReceiveData): Removed assertion that checks
        the ResourceHandle, since we can't do that any more.
        (WebCore::IconLoader::didFinishLoading): Changed to call finishLoading
        so we can share code with the new didReceiveResponse code path.
        (WebCore::IconLoader::finishLoading): Moved code here from the
        didFinishLoading callback. Pass a URL when calling
        commitIconURLToIconDatabase. Call notifyIconChanged directly here
        instead of using a separate function. Call clearLoadingState to
        share more code with stopLoading.
        (WebCore::IconLoader::clearLoadingState): Added.

        * loader/mac/IconLoaderMac.mm: Removed.
        * WebCore.xcodeproj/project.pbxproj: Removed IconLoaderMac.mm.

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

WebCore/ChangeLog
WebCore/WebCore.xcodeproj/project.pbxproj
WebCore/loader/icon/IconLoader.cpp
WebCore/loader/icon/IconLoader.h
WebCore/loader/mac/IconLoaderMac.mm [deleted file]
WebCore/page/Frame.cpp
WebCore/page/Frame.h

index 3b223632cdb888c00ed9c2ced744f2d25b489661..c8e1d21ac0ec3ef1c865895ca8e580751098db9e 100644 (file)
@@ -1,3 +1,51 @@
+2006-11-07  Darin Adler  <darin@apple.com>
+
+        Reviewed by Brady.
+
+        - fix <rdar://problem/4752069> 9A274: World of Warcraft Launcher
+          crashes on launch in WebCore::ResourceLoader::start
+
+        No layout test, because this depends on cached icons, although there's
+        perhaps a way to write a test for it with some further ingenuity.
+
+        * loader/icon/IconLoader.h: Make IconLoader inherit from Noncopyable
+        to make explicit the fact that it can't be successfully copied.
+        Remove notifyIconChanged function and put the contents in the caller.
+        This eliminates the need for IconLoaderMac.mm. Added finishedLoading
+        and clearLoadingState functions to share code. Removed m_url, since the
+        resource handle already stores the URL. Renamed m_resourceLoader to
+        m_handle to reflect the class's name change. Removed the 4096-byte
+        inline buffer from m_data, since the malloc savings is not sufficient
+        to offset the additional memory use. Removed m_httpStatusCode because
+        we can instead cancel the load when we get a status code that reflects
+        failure. Added m_loadIsInProgress boolean because we need to detect
+        loads that complete during the ResourceHandle::create function call.
+
+        * loader/icon/IconLoader.cpp:
+        (WebCore::IconLoader::IconLoader): Initialize m_loadIsInProgress.
+        Don't initialize m_httpStatusCode.
+        (WebCore::IconLoader::~IconLoader): Updated for name change.
+        (WebCore::IconLoader::startLoading): Added code to use the
+        m_loadIsInProgress flag to detect if the load completed while inside
+        the ResourceHandle::create function. Removed code to set m_url.
+        (WebCore::IconLoader::stopLoading): Call clearLoadingState to share
+        more code.
+        (WebCore::IconLoader::didReceiveResponse): Kill the ResourceHandle
+        and finish loading if the HTTP status code indicates failure.
+        (WebCore::IconLoader::didReceiveData): Removed assertion that checks
+        the ResourceHandle, since we can't do that any more.
+        (WebCore::IconLoader::didFinishLoading): Changed to call finishLoading
+        so we can share code with the new didReceiveResponse code path.
+        (WebCore::IconLoader::finishLoading): Moved code here from the
+        didFinishLoading callback. Pass a URL when calling
+        commitIconURLToIconDatabase. Call notifyIconChanged directly here
+        instead of using a separate function. Call clearLoadingState to
+        share more code with stopLoading.
+        (WebCore::IconLoader::clearLoadingState): Added.
+
+        * loader/mac/IconLoaderMac.mm: Removed.
+        * WebCore.xcodeproj/project.pbxproj: Removed IconLoaderMac.mm.
+
 2006-11-06  David Harrison  <harrison@apple.com>
 
         Reviewed by Darin.
index 57ba006de7cbb2c3819257a71b3afb08fe2bc660..f7ab10e2cb0db43565f25cf31aa83728c26e4f92 100644 (file)
                513F14530AB634C400094DDF /* IconLoader.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 513F14510AB634C400094DDF /* IconLoader.cpp */; };
                513F14540AB634C400094DDF /* IconLoader.h in Headers */ = {isa = PBXBuildFile; fileRef = 513F14520AB634C400094DDF /* IconLoader.h */; };
                5186C0560A9C21470034FE94 /* IconDataCache.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 5186C0550A9C21470034FE94 /* IconDataCache.cpp */; };
-               51A1D2C60AB69120000D732C /* IconLoaderMac.mm in Sources */ = {isa = PBXBuildFile; fileRef = 51A1D2C50AB69120000D732C /* IconLoaderMac.mm */; };
                51D3EA160A3D24D300BADA35 /* SQLDatabase.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 51D3EA130A3D24D300BADA35 /* SQLDatabase.cpp */; };
                51D3EA170A3D24D300BADA35 /* SQLDatabase.h in Headers */ = {isa = PBXBuildFile; fileRef = 51D3EA140A3D24D300BADA35 /* SQLDatabase.h */; };
                51D3EA180A3D24D300BADA35 /* SQLStatement.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 51D3EA150A3D24D300BADA35 /* SQLStatement.cpp */; };
                5150C2A10702629000AF642C /* WebDashboardRegion.h */ = {isa = PBXFileReference; fileEncoding = 30; indentWidth = 4; lastKnownFileType = sourcecode.c.h; path = WebDashboardRegion.h; sourceTree = "<group>"; tabWidth = 8; usesTabs = 0; };
                5150C2A50702629800AF642C /* WebDashboardRegion.m */ = {isa = PBXFileReference; fileEncoding = 30; indentWidth = 4; lastKnownFileType = sourcecode.c.objc; path = WebDashboardRegion.m; sourceTree = "<group>"; tabWidth = 8; usesTabs = 0; };
                5186C0550A9C21470034FE94 /* IconDataCache.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = IconDataCache.cpp; sourceTree = "<group>"; };
-               51A1D2C50AB69120000D732C /* IconLoaderMac.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = IconLoaderMac.mm; sourceTree = "<group>"; };
                51D3EA130A3D24D300BADA35 /* SQLDatabase.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = SQLDatabase.cpp; sourceTree = "<group>"; };
                51D3EA140A3D24D300BADA35 /* SQLDatabase.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = SQLDatabase.h; sourceTree = "<group>"; };
                51D3EA150A3D24D300BADA35 /* SQLStatement.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = SQLStatement.cpp; sourceTree = "<group>"; };
                                654F68870AF1B7C50065BDD6 /* CachedResourceMac.mm */,
                                656D371F0ADBA5DE00A4554D /* DocumentLoaderMac.mm */,
                                656D37250ADBA5DE00A4554D /* FrameLoaderMac.mm */,
-                               51A1D2C50AB69120000D732C /* IconLoaderMac.mm */,
                                93A1EAA70A563508006960A0 /* ImageDocumentMac.h */,
                                93A1EA9F0A5634C9006960A0 /* ImageDocumentMac.mm */,
                                F587850302DE375901EA4122 /* LoaderFunctionsMac.mm */,
                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 = "";
+                       shouldCheckCompatibility = 1;
                        targets = (
                                93F198A508245E59001E9ABC /* WebCore */,
                                DD041FBE09D9DDBE0010AF2A /* Derived Sources */,
                                85E9E0A60AB3A0C700069CD0 /* DOMXPathResult.mm in Sources */,
                                06E81EEC0AB5DA9700C87837 /* LocalCurrentGraphicsContext.mm in Sources */,
                                513F14530AB634C400094DDF /* IconLoader.cpp in Sources */,
-                               51A1D2C60AB69120000D732C /* IconLoaderMac.mm in Sources */,
                                066C772D0AB603D200238CC4 /* FileChooserMac.mm in Sources */,
                                066C77300AB603FD00238CC4 /* RenderFileUploadControl.cpp in Sources */,
                                066C773E0AB6053F00238CC4 /* IconMac.mm in Sources */,
index 55f80cbcd64807fa027cbf97c2c235df8a68a291..048a8dc2f5c13e76495448c34fef868104122ade 100644 (file)
@@ -28,6 +28,7 @@
 
 #include "Document.h"
 #include "Frame.h"
+#include "FrameLoader.h"
 #include "IconDatabase.h"
 #include "Logging.h"
 #include "ResourceHandle.h"
@@ -38,9 +39,11 @@ using namespace std;
 
 namespace WebCore {
 
+const size_t defaultBufferSize = 4096; // bigger than most icons
+
 IconLoader::IconLoader(Frame* frame)
     : m_frame(frame)
-    , m_httpStatusCode(0)
+    , m_loadIsInProgress(false)
 {
 }
 
@@ -51,88 +54,90 @@ auto_ptr<IconLoader> IconLoader::create(Frame* frame)
 
 IconLoader::~IconLoader()
 {
-    if (m_resourceLoader)
-        m_resourceLoader->kill();
+    if (m_handle)
+        m_handle->kill();
 }
 
 void IconLoader::startLoading()
 {    
-    if (m_resourceLoader)
+    if (m_handle)
         return;
-    
-    m_httpStatusCode = 0;
-    
-    // FIXME - http://bugs.webkit.org/show_bug.cgi?id=10902
-    // Once the loader infrastructure will cleanly let us load an icon without a DocLoader, we can implement this
-    // A frame may be documentless - one example is viewing a PDF directly.  Until the above FIXME is resolved,
-    // we must bail out early when we have no document
+
+    // FIXME: http://bugs.webkit.org/show_bug.cgi?id=10902
+    // Once ResourceHandle will load without a DocLoader, we can remove this check.
+    // A frame may be documentless - one example is a frame containing only a PDF.
     if (!m_frame->document()) {
         LOG(IconDatabase, "Documentless-frame - icon won't be loaded");
         return;
-    } 
-    
-    m_url = m_frame->iconURL();
-    m_resourceLoader = ResourceHandle::create(m_url, this, m_frame->document()->docLoader());
+    }
+
+    // Set flag so we can detect the case where the load completes before
+    // ResourceHandle::create returns.
+    m_loadIsInProgress = true;
+    m_buffer.reserveCapacity(defaultBufferSize);
 
-    if (!m_resourceLoader)
+    RefPtr<ResourceHandle> loader = ResourceHandle::create(m_frame->iconURL(),
+        this, m_frame->document()->docLoader());
+    if (!loader)
         LOG_ERROR("Failed to start load for icon at url %s", m_frame->iconURL().url().ascii());
+
+    // Store the handle so we can cancel the load if stopLoading is called later.
+    // But only do it if the load hasn't already completed.
+    if (m_loadIsInProgress)
+        m_handle = loader.release();
 }
 
 void IconLoader::stopLoading()
 {
-    if (m_resourceLoader)
-        m_resourceLoader->kill();
-    m_resourceLoader = 0;
-    m_data.clear();
+    if (m_handle)
+        m_handle->kill();
+    clearLoadingState();
 }
 
-void IconLoader::didReceiveResponse(ResourceHandle* resourceLoader, const ResourceResponse& response)
+void IconLoader::didReceiveResponse(ResourceHandle* handle, const ResourceResponse& response)
 {
-    ASSERT(resourceLoader);
-    m_httpStatusCode = response.httpStatusCode();
+    // If we got a status code indicating an invalid response, then lets
+    // ignore the data and do not try to decode the error page as an icon.
+    int status = response.httpStatusCode();
+    if (status && (status < 200 || status > 299)) {
+        KURL iconURL = handle->url();
+        handle->kill();
+        finishLoading(iconURL);
+    }
 }
 
-void IconLoader::didReceiveData(ResourceHandle* resourceLoader, const char* data, int size)
+void IconLoader::didReceiveData(ResourceHandle*, const char* data, int size)
 {
-    ASSERT(resourceLoader == m_resourceLoader);
-    ASSERT(data);
-    ASSERT(size > -1);
-    m_data.append(data, size);
+    ASSERT(data || size == 0);
+    ASSERT(size >= 0);
+    m_buffer.append(data, size);
 }
 
-void IconLoader::didFinishLoading(ResourceHandle* resourceLoader)
+void IconLoader::didFinishLoading(ResourceHandle* handle)
 {
-    ASSERT(resourceLoader == m_resourceLoader);
-
-    const char* data;
-    int size;
-    
-    // If we logged an HTTP response, only set the icon data if it was a valid response
-    if (m_httpStatusCode && (m_httpStatusCode < 200 || m_httpStatusCode > 299)) {
-        data = 0;
-        size = 0;
-    } else {
-        data = m_data.data();
-        size = m_data.size();
-    }
-        
-    IconDatabase* iconDatabase = IconDatabase::sharedIconDatabase();
-    ASSERT(iconDatabase);
-    
-    if (data)
-        iconDatabase->setIconDataForIconURL(data, size, m_url.url());
-    else
-        iconDatabase->setHaveNoIconForIconURL(m_url.url());
-        
-    // Tell the frame to map its url(s) to its iconURL in the database
-    m_frame->commitIconURLToIconDatabase();
-    
-    // Send the notification to the app that this icon is finished loading
-    notifyIconChanged(m_url);
-
-    // ResourceLoaders delete themselves after they deliver their last data, so we can just forget about it
-    m_resourceLoader = 0;
-    m_data.clear();
+    finishLoading(handle->url());
+}
+
+void IconLoader::finishLoading(const KURL& iconURL)
+{
+    IconDatabase::sharedIconDatabase()->setIconDataForIconURL(m_buffer.data(), m_buffer.size(), iconURL.url());
+
+    // Tell the frame to map its URL(s) to its iconURL in the database.
+    m_frame->commitIconURLToIconDatabase(iconURL);
+
+    // Send the notification to the app that this icon is finished loading.
+#if PLATFORM(MAC) // turn this on for other platforms when FrameLoader is deployed more fully
+    m_frame->loader()->notifyIconChanged();
+#endif
+
+    clearLoadingState();
+}
+
+void IconLoader::clearLoadingState()
+{
+    m_handle = 0;
+    m_buffer.clear();
+    m_loadIsInProgress = false;
 }
 
 }
index 4af0d7880cc0f0998dd5d88829557fba6f4ca20a..ada6103ea5fc8433e6bccde0dedbad2b94070460 100644 (file)
 
 #include "KURL.h"
 #include "ResourceHandleClient.h"
+#include <wtf/Noncopyable.h>
 #include <wtf/Vector.h>
 
 namespace WebCore {
 
 class Frame;
 
-class IconLoader : public ResourceHandleClient {
+class IconLoader : public ResourceHandleClient, Noncopyable {
 public:
     static std::auto_ptr<IconLoader> create(Frame*);
     ~IconLoader();
@@ -43,22 +44,21 @@ public:
     void stopLoading();
 
 private:
+    IconLoader(Frame*);
+
     virtual void didReceiveResponse(ResourceHandle*, const ResourceResponse&);
     virtual void didReceiveData(ResourceHandle*, const char*, int);
     virtual void didFinishLoading(ResourceHandle*);
 
-    IconLoader(Frame*);
-    
-    void notifyIconChanged(const KURL&);
+    void finishLoading(const KURL&);
+    void clearLoadingState();
 
-    KURL m_url;
-    RefPtr<ResourceHandle> m_resourceLoader;
     Frame* m_frame;
-    
-    static const int IconLoaderDefaultBuffer = 4096;
-    Vector<char, IconLoaderDefaultBuffer> m_data;
-    int m_httpStatusCode;
-}; // class Iconloader
+
+    RefPtr<ResourceHandle> m_handle;
+    Vector<char> m_buffer;
+    bool m_loadIsInProgress;
+}; // class IconLoader
 
 } // namespace WebCore
 
diff --git a/WebCore/loader/mac/IconLoaderMac.mm b/WebCore/loader/mac/IconLoaderMac.mm
deleted file mode 100644 (file)
index 43948a8..0000000
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
- * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
- * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
- * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
- */
-#include "config.h"
-#include "IconLoader.h"
-
-#include "FrameLoader.h"
-#include "Frame.h"
-
-namespace WebCore {
-
-void IconLoader::notifyIconChanged(const KURL&)
-{
-    // FIXME: Change caller to not send the URL?
-    // FIXME: Move this out of Mac-specific into platform-independent.
-    m_frame->loader()->notifyIconChanged();
-}
-
-}
index ec31fb0d3b1112df89b4064af99e1a87889068d6..27aec41dbe334dc5d0245266feef53162ac28746 100644 (file)
@@ -1040,7 +1040,7 @@ void Frame::endIfNotLoading()
         
         // If we already have an unexpired icon, we won't kick off a load but we *will* map the appropriate URLs to it
         if (sharedIconDatabase->hasEntryForIconURL(url) && !isLoadTypeReload() && !sharedIconDatabase->isIconExpiredForIconURL(url)) {
-            commitIconURLToIconDatabase();
+            commitIconURLToIconDatabase(iconURL());
             return;
         }
         
@@ -1050,10 +1050,8 @@ void Frame::endIfNotLoading()
     }
 }
 
-void Frame::commitIconURLToIconDatabase()
+void Frame::commitIconURLToIconDatabase(const KURL& icon)
 {
-    KURL icon = iconURL();
-    
     IconDatabase* iconDatabase = IconDatabase::sharedIconDatabase();
     ASSERT(iconDatabase);
     iconDatabase->setIconURLForPageURL(icon.url(), this->url().url());
index 28df3e384bc452b001a58908e0d58fed939d0ccf..302651e18da3bc1a43946f85f727b55269d99e37 100644 (file)
@@ -390,7 +390,7 @@ public:
     void didExplicitOpen();
 
     KURL iconURL();
-    void commitIconURLToIconDatabase();
+    void commitIconURLToIconDatabase(const KURL&);
 
     void setAutoLoadImages(bool enable);
     bool autoLoadImages() const;