Changeset 21732 in webkit


Ignore:
Timestamp:
May 24, 2007 3:56:33 PM (17 years ago)
Author:
darin
Message:

Reviewed by Hyatt.

  • fix <rdar://problem/5226451> REGRESSION (21618): Mail block quotes are missing the lines on the side

This patch fixes three problems:

1) makes the "skip canLoad check" boolean also skip the check

that prevents any loads while the document is in provisional
state; this is the proximate cause of the bug

  • loader/SubresourceLoader.cpp: (WebCore::SubresourceLoader::create): Don't check the frame's state if skipCanLoadCheck is true.

2) moves the "skip canLoad check" boolean to the Request object;

the old implementation would cause that flag to affect the
new request we served, which might not be the resource with
that flag set

3) fixes error-handling code path that would leak requests

  • html/HTMLImageLoader.cpp: (WebCore::HTMLImageLoader::updateFromElement): Pass false to the CachedImage constructor to indicate we are not making this object for the cache.
  • loader/Cache.cpp: (WebCore::createResource): Pass true to the CachedImage constructor to indicate we are making this object for the cache. (WebCore::Cache::requestResource): Add new code that assumes the object will already have the inCache bit set, and that will delete the object and return 0 if the cache is disabled and the load failed.
  • loader/CachedImage.h:
  • loader/CachedImage.cpp: (WebCore::CachedImage::CachedImage): Added a forCache boolean parameter. Always false for the constructor that's only used outside the cache code, and passed in as a boolean for the constructor that's used both in cache and outside cache.
  • loader/CachedResource.h:
  • loader/CachedResource.cpp: (WebCore::CachedResource::CachedResource): Added a forCache boolean parameter that determines the initial state of the m_inCache flag. This is needed to prevent a resource from being destroyed if an error occurs during the initial load.
  • loader/DocLoader.cpp: (WebCore::DocLoader::requestCSSStyleSheet): Added FIXME. (WebCore::DocLoader::setLoadInProgress): Added null check.
  • loader/Request.h:
  • loader/Request.cpp: (WebCore::Request::Request): Added a shouldSkipCanLoadCheck boolean here, since we need to track this for each request.
  • loader/loader.h:
  • loader/loader.cpp: (WebCore::Loader::load): Pass the skipCanLoadCheck boolean to the Request constructor rather than to the servePendingRequests function. (WebCore::Loader::servePendingRequests): Add a loop so we can handle cases where the request fails immediately without leaking the request and thinking that we're loading forever.
Location:
trunk/WebCore
Files:
13 edited

Legend:

Unmodified
Added
Removed
  • trunk/WebCore/ChangeLog

    r21731 r21732  
     12007-05-24  Darin Adler  <darin@apple.com>
     2
     3        Reviewed by Hyatt.
     4
     5        - fix <rdar://problem/5226451> REGRESSION (21618): Mail block quotes
     6          are missing the lines on the side
     7
     8        This patch fixes three problems:
     9
     10        1) makes the "skip canLoad check" boolean also skip the check
     11           that prevents any loads while the document is in provisional
     12           state; this is the proximate cause of the bug
     13
     14        * loader/SubresourceLoader.cpp: (WebCore::SubresourceLoader::create):
     15        Don't check the frame's state if skipCanLoadCheck is true.
     16
     17        2) moves the "skip canLoad check" boolean to the Request object;
     18           the old implementation would cause that flag to affect the
     19           new request we served, which might not be the resource with
     20           that flag set
     21
     22        3) fixes error-handling code path that would leak requests
     23
     24        * html/HTMLImageLoader.cpp: (WebCore::HTMLImageLoader::updateFromElement):
     25        Pass false to the CachedImage constructor to indicate we are not making
     26        this object for the cache.
     27
     28        * loader/Cache.cpp:
     29        (WebCore::createResource): Pass true to the CachedImage constructor to
     30        indicate we are making this object for the cache.
     31        (WebCore::Cache::requestResource): Add new code that assumes the object
     32        will already have the inCache bit set, and that will delete the object
     33        and return 0 if the cache is disabled and the load failed.
     34
     35        * loader/CachedImage.h:
     36        * loader/CachedImage.cpp: (WebCore::CachedImage::CachedImage):
     37        Added a forCache boolean parameter. Always false for the constructor
     38        that's only used outside the cache code, and passed in as a boolean
     39        for the constructor that's used both in cache and outside cache.
     40
     41        * loader/CachedResource.h:
     42        * loader/CachedResource.cpp:
     43        (WebCore::CachedResource::CachedResource): Added a forCache boolean
     44        parameter that determines the initial state of the m_inCache flag.
     45        This is needed to prevent a resource from being destroyed if an
     46        error occurs during the initial load.
     47
     48        * loader/DocLoader.cpp:
     49        (WebCore::DocLoader::requestCSSStyleSheet): Added FIXME.
     50        (WebCore::DocLoader::setLoadInProgress): Added null check.
     51
     52        * loader/Request.h:
     53        * loader/Request.cpp: (WebCore::Request::Request):
     54        Added a shouldSkipCanLoadCheck boolean here, since we need to track
     55        this for each request.
     56
     57        * loader/loader.h:
     58        * loader/loader.cpp:
     59        (WebCore::Loader::load): Pass the skipCanLoadCheck boolean to the
     60        Request constructor rather than to the servePendingRequests function.
     61        (WebCore::Loader::servePendingRequests): Add a loop so we can handle
     62        cases where the request fails immediately without leaking the request
     63        and thinking that we're loading forever.
     64
    1652007-05-24  David Hyatt  <hyatt@apple.com>
    266
  • trunk/WebCore/html/HTMLImageLoader.cpp

    r20674 r21732  
    1 /**
    2  * This file is part of the DOM implementation for KDE.
    3  *
     1/*
    42 * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
    53 *           (C) 1999 Antti Koivisto (koivisto@kde.org)
    6  * Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
     4 * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
    75 *
    86 * This library is free software; you can redistribute it and/or
     
    2119 * Boston, MA 02111-1307, USA.
    2220 */
     21
    2322#include "config.h"
    2423#include "HTMLImageLoader.h"
     
    9695        if (m_loadManually) {
    9796            doc->docLoader()->setAutoLoadImages(false);
    98             newImage = new CachedImage(doc->docLoader(), parseURL(attr));
     97            newImage = new CachedImage(doc->docLoader(), parseURL(attr), false /* not for cache */);
    9998            newImage->setLoading(true);
    10099            doc->docLoader()->m_docResources.set(newImage->url(), newImage);
  • trunk/WebCore/loader/Cache.cpp

    r21009 r21732  
    33    Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
    44    Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
    5     Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
     5    Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
    66
    77    This library is free software; you can redistribute it and/or
     
    1919    the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
    2020    Boston, MA 02111-1307, USA.
    21 
    22     This class provides all functionality needed for loading images, style sheets and html
    23     pages from the web. It has a memory cache for these objects.
    2421*/
    2522
     
    6562    case CachedResource::ImageResource:
    6663        // User agent images need to null check the docloader.  No other resources need to.
    67         return new CachedImage(docLoader, url.url());
     64        return new CachedImage(docLoader, url.url(), true /* for cache */);
    6865    case CachedResource::CSSStyleSheet:
    6966        return new CachedCSSStyleSheet(docLoader, url.url(), *charset, skipCanLoadCheck);
     
    107104        }
    108105
    109         // The resource does not exist.  Create it.
     106        // The resource does not exist. Create it.
    110107        resource = createResource(type, docLoader, url, charset, skipCanLoadCheck);
    111108        ASSERT(resource);
    112         resource->setInCache(!disabled());
     109        ASSERT(resource->inCache());
    113110        if (!disabled())
    114111            m_resources.set(url.url(), resource);  // The size will be added in later once the resource is loaded and calls back to us with the new size.
     112        else {
     113            // Kick the resource out of the cache, because the cache is disabled.
     114            resource->setInCache(false);
     115            if (resource->errorOccurred()) {
     116                // We don't support immediate loads, but we do support immediate failure.
     117                // In that case we should to delete the resource now and return 0 because otherwise
     118                // it would leak if no ref/deref was ever done on it.
     119                delete resource;
     120                return 0;
     121            }
     122        }
    115123    }
    116124
  • trunk/WebCore/loader/CachedImage.cpp

    r20674 r21732  
    11/*
    2     This file is part of the KDE libraries
    3 
    42    Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
    53    Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
    64    Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
    75    Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
    8     Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
     6    Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
    97
    108    This library is free software; you can redistribute it and/or
     
    2220    the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
    2321    Boston, MA 02111-1307, USA.
    24 
    25     This class provides all functionality needed for loading images, style sheets and html
    26     pages from the web. It has a memory cache for these objects.
    2722*/
    2823
     
    5247namespace WebCore {
    5348
    54 CachedImage::CachedImage(DocLoader* docLoader, const String& url)
    55     : CachedResource(url, ImageResource)
     49CachedImage::CachedImage(DocLoader* docLoader, const String& url, bool forCache)
     50    : CachedResource(url, ImageResource, forCache)
    5651{
    5752    m_image = 0;
     
    6560
    6661CachedImage::CachedImage(Image* image)
    67     : CachedResource(String(), ImageResource)
     62    : CachedResource(String(), ImageResource, false /* not for cache */)
    6863{
    6964    m_image = image;
  • trunk/WebCore/loader/CachedImage.h

    r20674 r21732  
    11/*
    2     This file is part of the KDE libraries
    3 
    42    Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
    53    Copyright (C) 2001 Dirk Mueller <mueller@kde.org>
    64    Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
    7     Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
     5    Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
    86
    97    This library is free software; you can redistribute it and/or
     
    2119    the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
    2220    Boston, MA 02111-1307, USA.
    23 
    24     This class provides all functionality needed for loading images, style sheets and html
    25     pages from the web. It has a memory cache for these objects.
    2621*/
    2722
     
    4237class CachedImage : public CachedResource, public ImageObserver {
    4338public:
    44     CachedImage(DocLoader*, const String& url);
     39    CachedImage(DocLoader*, const String& url, bool forCache);
    4540    CachedImage(Image*);
    4641    virtual ~CachedImage();
  • trunk/WebCore/loader/CachedResource.cpp

    r20693 r21732  
    11/*
    2     This file is part of the KDE libraries
    3 
    42    Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
    53    Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
    64    Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
    75    Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
    8     Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
     6    Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
    97
    108    This library is free software; you can redistribute it and/or
     
    2220    the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
    2321    Boston, MA 02111-1307, USA.
    24 
    25     This class provides all functionality needed for loading images, style sheets and html
    26     pages from the web. It has a memory cache for these objects.
    2722*/
    2823
     
    3833namespace WebCore {
    3934
    40 CachedResource::CachedResource(const String& URL, Type type)
     35CachedResource::CachedResource(const String& URL, Type type, bool forCache)
     36    : m_inCache(forCache)
    4137{
    4238    m_url = URL;
     
    4440    m_status = Pending;
    4541    m_encodedSize = 0;
    46     m_inCache = false;
    4742    m_request = 0;
    4843
  • trunk/WebCore/loader/CachedResource.h

    r20674 r21732  
    11/*
    2     This file is part of the KDE libraries
    3 
    42    Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
    53    Copyright (C) 2001 Dirk Mueller <mueller@kde.org>
    64    Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
    7     Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
     5    Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
    86
    97    This library is free software; you can redistribute it and/or
     
    2119    the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
    2220    Boston, MA 02111-1307, USA.
    23 
    24     This class provides all functionality needed for loading images, style sheets and html
    25     pages from the web. It has a memory cache for these objects.
    2621*/
    2722
     
    6762    };
    6863
    69     CachedResource(const String& URL, Type type);
     64    CachedResource(const String& URL, Type, bool forCache = true);
    7065    virtual ~CachedResource();
    7166
  • trunk/WebCore/loader/DocLoader.cpp

    r21245 r21732  
    9393CachedCSSStyleSheet* DocLoader::requestCSSStyleSheet(const String& url, const String& charset, bool isUserStyleSheet)
    9494{
     95    // FIXME: Passing true for "skipCanLoadCheck" here in the isUserStyleSheet case  won't have any effect
     96    // if this resource is already in the cache. It's theoretically possible that what's in the cache already
     97    // is a load that failed because of the canLoad check. Probably not an issue in practice.
    9598    return static_cast<CachedCSSStyleSheet*>(requestResource(CachedResource::CSSStyleSheet, url, &charset, isUserStyleSheet));
    9699}
     
    175178{
    176179    m_loadInProgress = load;
    177     if (!load)
     180    if (!load && m_frame)
    178181        m_frame->loader()->loadDone();
    179182}
  • trunk/WebCore/loader/Request.cpp

    r15269 r21732  
    11/*
    2     This file is part of the KDE libraries
    3 
    42    Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
    53    Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
    64    Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
    75    Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
    8     Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
     6    Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
    97
    108    This library is free software; you can redistribute it and/or
     
    2220    the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
    2321    Boston, MA 02111-1307, USA.
    24 
    25     This class provides all functionality needed for loading images, style sheets and html
    26     pages from the web. It has a memory cache for these objects.
    2722*/
    2823
     
    3429namespace WebCore {
    3530
    36 Request::Request(DocLoader* docLoader, CachedResource* object, bool incremental)
     31Request::Request(DocLoader* docLoader, CachedResource* object, bool incremental, bool shouldSkipCanLoadCheck)
    3732    : m_object(object)
    3833    , m_docLoader(docLoader)
    3934    , m_incremental(incremental)
    4035    , m_multipart(false)
     36    , m_shouldSkipCanLoadCheck(shouldSkipCanLoadCheck)
    4137{
    4238    m_object->setRequest(this);
  • trunk/WebCore/loader/Request.h

    r18874 r21732  
    11/*
    2     This file is part of the KDE libraries
    3 
    42    Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
    53    Copyright (C) 2001 Dirk Mueller <mueller@kde.org>
    64    Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
    7     Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
     5    Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
    86
    97    This library is free software; you can redistribute it and/or
     
    2119    the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
    2220    Boston, MA 02111-1307, USA.
    23 
    24     This class provides all functionality needed for loading images, style sheets and html
    25     pages from the web. It has a memory cache for these objects.
    2621*/
    2722
     
    3833    class Request {
    3934    public:
    40         Request(DocLoader*, CachedResource*, bool incremental);
     35        Request(DocLoader*, CachedResource*, bool incremental, bool skipCanLoadCheck);
    4136        ~Request();
    4237       
     
    5146        void setIsMultipart(bool b = true) { m_multipart = b; }
    5247
     48        bool shouldSkipCanLoadCheck() const { return m_shouldSkipCanLoadCheck; }
     49
    5350    private:
    5451        Vector<char> m_buffer;
     
    5754        bool m_incremental;
    5855        bool m_multipart;
     56        bool m_shouldSkipCanLoadCheck;
    5957    };
    6058
  • trunk/WebCore/loader/SubresourceLoader.cpp

    r20740 r21732  
    11/*
    2  * Copyright (C) 2006 Apple Computer, Inc. All rights reserved.
     2 * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    8989
    9090    FrameLoader* fl = frame->loader();
    91     if (fl->state() == FrameStateProvisional)
     91    if (!skipCanLoadCheck && fl->state() == FrameStateProvisional)
    9292        return 0;
    9393
     
    9595
    9696    if (!skipCanLoadCheck
    97     && FrameLoader::restrictAccessToLocal()
    98     && !FrameLoader::canLoad(request.url(), frame->document())) {
     97            && FrameLoader::restrictAccessToLocal()
     98            && !FrameLoader::canLoad(request.url(), frame->document())) {
    9999        FrameLoader::reportLocalLoadFailed(frame->page(), request.url().url());
    100100        return 0;
  • trunk/WebCore/loader/loader.cpp

    r21536 r21732  
    11/*
    2     This file is part of the KDE libraries
    3 
    42    Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
    53    Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
    64    Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
    75    Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
    8     Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
     6    Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
    97
    108    This library is free software; you can redistribute it and/or
     
    2220    the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
    2321    Boston, MA 02111-1307, USA.
    24 
    25     This class provides all functionality needed for loading images, style sheets and html
    26     pages from the web. It has a memory cache for these objects.
    2722*/
    2823
     
    6055{
    6156    ASSERT(dl);
    62     Request* req = new Request(dl, object, incremental);
     57    Request* req = new Request(dl, object, incremental, skipCanLoadCheck);
    6358    m_requestsPending.append(req);
    6459    dl->incrementRequestCount();
    65     servePendingRequests(skipCanLoadCheck);
    66 }
    67 
    68 void Loader::servePendingRequests(bool skipCanLoadCheck)
    69 {
    70     if (m_requestsPending.count() == 0)
    71         return;
    72 
    73     // get the first pending request
    74     Request* req = m_requestsPending.take(0);
    75     DocLoader* dl = req->docLoader();
    76     dl->decrementRequestCount();
    77 
    78     ResourceRequest request(req->cachedResource()->url());
    79 
    80     if (!req->cachedResource()->accept().isEmpty())
    81         request.setHTTPAccept(req->cachedResource()->accept());
    82 
    83     KURL r = dl->doc()->URL();
    84     if (r.protocol().startsWith("http") && r.path().isEmpty())
    85         r.setPath("/");
    86     request.setHTTPReferrer(r.url());
    87     DeprecatedString domain = r.host();
    88     if (dl->doc()->isHTMLDocument())
    89         domain = static_cast<HTMLDocument*>(dl->doc())->domain().deprecatedString();
    90    
    91     RefPtr<SubresourceLoader> loader = SubresourceLoader::create(dl->doc()->frame(), this, request, skipCanLoadCheck);
    92 
    93     if (loader) {
    94         m_requestsLoading.add(loader.release(), req);
    95         dl->incrementRequestCount();
     60    servePendingRequests();
     61}
     62
     63void Loader::servePendingRequests()
     64{
     65    while (!m_requestsPending.isEmpty()) {
     66        // get the first pending request
     67        Request* req = m_requestsPending.take(0);
     68        DocLoader* dl = req->docLoader();
     69        dl->decrementRequestCount();
     70
     71        ResourceRequest request(req->cachedResource()->url());
     72
     73        if (!req->cachedResource()->accept().isEmpty())
     74            request.setHTTPAccept(req->cachedResource()->accept());
     75
     76        KURL r = dl->doc()->URL();
     77        if (r.protocol().startsWith("http") && r.path().isEmpty())
     78            r.setPath("/");
     79        request.setHTTPReferrer(r.url());
     80        DeprecatedString domain = r.host();
     81        if (dl->doc()->isHTMLDocument())
     82            domain = static_cast<HTMLDocument*>(dl->doc())->domain().deprecatedString();
     83       
     84        RefPtr<SubresourceLoader> loader = SubresourceLoader::create(dl->doc()->frame(),
     85            this, request, req->shouldSkipCanLoadCheck());
     86
     87        if (loader) {
     88            m_requestsLoading.add(loader.release(), req);
     89            dl->incrementRequestCount();
     90            break;
     91        }
     92
     93        dl->setLoadInProgress(true);
     94        req->cachedResource()->error();
     95        dl->setLoadInProgress(false);
     96
     97        delete req;
    9698    }
    9799}
  • trunk/WebCore/loader/loader.h

    r21245 r21732  
    11/*
    2     This file is part of the KDE libraries
    3 
    42    Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
    53    Copyright (C) 2001 Dirk Mueller <mueller@kde.org>
    6     Copyright (C) 2004, 2006 Apple Computer, Inc.
     4    Copyright (C) 2004, 2006, 2007 Apple Inc. All rights reserved.
    75
    86    This library is free software; you can redistribute it and/or
     
    2018    the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
    2119    Boston, MA 02111-1307, USA.
    22 
    23     This class provides all functionality needed for loading images, style sheets and html
    24     pages from the web. It has a memory cache for these objects.
    2520*/
    2621
     
    2823#define loader_h
    2924
     25#include "DeprecatedPtrList.h"
    3026#include "SubresourceLoaderClient.h"
    3127#include <wtf/HashMap.h>
    32 #include "DeprecatedPtrList.h"
    33 
    34 #ifdef __OBJC__
    35 @class NSData;
    36 #else
    37 class NSData;
    38 #endif
    3928
    4029namespace WebCore {
     
    4332    class DocLoader;
    4433    class Request;
    45     class String;
    4634
    4735    class Loader : private SubresourceLoaderClient {
     
    5341
    5442        void cancelRequests(DocLoader*);
     43
    5544    private:
    5645        virtual void didReceiveResponse(SubresourceLoader*, const ResourceResponse&);
     
    6049        void didFail(SubresourceLoader*, bool cancelled = false);
    6150
    62         void servePendingRequests(bool skipCanLoadCheck = false);
     51        void servePendingRequests();
    6352
    6453        DeprecatedPtrList<Request> m_requestsPending;
Note: See TracChangeset for help on using the changeset viewer.