Changeset 121929 in webkit


Ignore:
Timestamp:
Jul 5, 2012 5:20:44 PM (12 years ago)
Author:
benjamin@webkit.org
Message:

Double release of resources if the load is canceled in a callback of ResourceLoader::didFinishLoading
https://bugs.webkit.org/show_bug.cgi?id=90431

Patch by Benjamin Poulain <bpoulain@apple.com> on 2012-07-05
Reviewed by Anders Carlsson.

Source/WebCore:

In ResourceLoader::didFinishLoadingOnePart(), we invoke didFinishLoad() on the WebKit client. If WebKit
causes the current frame to cancel the load synchronously, the resources are already freed when
ResourceLoader::didFinishLoadingOnePart() ends.
When ResourceLoader::didFinishLoading() subsequently invokes releaseResources(), we are releasing the
resources a second time.

This patch add a second check for cancellation after invoking ResourceLoader::didFinishLoadingOnePart() to
avoid such issues.

The previous check at the beginning of ResourceLoader::didFinishLoading() has been removed because it is
redundant with ResourceLoader::didFinishLoadingOnePart().

  • loader/ResourceLoader.cpp:

(WebCore::ResourceLoader::didFinishLoading):
(WebCore::ResourceLoader::didFinishLoadingOnePart):

Tools:

Add a Mac API test.

  • TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
  • TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.html: Added.
  • TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.mm: Added.

(-[CancelLoadFromResourceLoadDelegate webView:resource:didFinishLoadingFromDataSource:]):
(-[CancelLoadFromResourceLoadDelegateFrameLoadDelegate webView:didFinishLoadForFrame:]):
(TestWebKitAPI):
(TestWebKitAPI::TEST):

Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r121926 r121929  
     12012-07-05  Benjamin Poulain  <bpoulain@apple.com>
     2
     3        Double release of resources if the load is canceled in a callback of ResourceLoader::didFinishLoading
     4        https://bugs.webkit.org/show_bug.cgi?id=90431
     5
     6        Reviewed by Anders Carlsson.
     7
     8        In ResourceLoader::didFinishLoadingOnePart(), we invoke didFinishLoad() on the WebKit client. If WebKit
     9        causes the current frame to cancel the load synchronously, the resources are already freed when
     10        ResourceLoader::didFinishLoadingOnePart() ends.
     11        When ResourceLoader::didFinishLoading() subsequently invokes releaseResources(), we are releasing the
     12        resources a second time.
     13
     14        This patch add a second check for cancellation after invoking ResourceLoader::didFinishLoadingOnePart() to
     15        avoid such issues.
     16
     17        The previous check at the beginning of ResourceLoader::didFinishLoading() has been removed because it is
     18        redundant with ResourceLoader::didFinishLoadingOnePart().
     19
     20        * loader/ResourceLoader.cpp:
     21        (WebCore::ResourceLoader::didFinishLoading):
     22        (WebCore::ResourceLoader::didFinishLoadingOnePart):
     23
    1242012-07-05  Simon Fraser  <simon.fraser@apple.com>
    225
  • trunk/Source/WebCore/loader/ResourceLoader.cpp

    r121592 r121929  
    289289void ResourceLoader::didFinishLoading(double finishTime)
    290290{
    291     // If load has been cancelled after finishing (which could happen with a
     291    didFinishLoadingOnePart(finishTime);
     292
     293    // If the load has been cancelled by a delegate in response to didFinishLoad(), do not release
     294    // the resources a second time, they have been released by cancel.
     295    if (m_cancelled)
     296        return;
     297    releaseResources();
     298}
     299
     300void ResourceLoader::didFinishLoadingOnePart(double finishTime)
     301{
     302    // If load has been cancelled after finishing (which could happen with a
    292303    // JavaScript that changes the window location), do nothing.
    293     if (m_cancelled)
    294         return;
    295     ASSERT(!m_reachedTerminalState);
    296 
    297     didFinishLoadingOnePart(finishTime);
    298     releaseResources();
    299 }
    300 
    301 void ResourceLoader::didFinishLoadingOnePart(double finishTime)
    302 {
    303304    if (m_cancelled)
    304305        return;
  • trunk/Tools/ChangeLog

    r121915 r121929  
     12012-07-05  Benjamin Poulain  <bpoulain@apple.com>
     2
     3        Double release of resources if the load is canceled in a callback of ResourceLoader::didFinishLoading
     4        https://bugs.webkit.org/show_bug.cgi?id=90431
     5
     6        Reviewed by Anders Carlsson.
     7
     8        Add a Mac API test.
     9
     10        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     11        * TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.html: Added.
     12        * TestWebKitAPI/Tests/mac/CancelLoadFromResourceLoadDelegate.mm: Added.
     13        (-[CancelLoadFromResourceLoadDelegate webView:resource:didFinishLoadingFromDataSource:]):
     14        (-[CancelLoadFromResourceLoadDelegateFrameLoadDelegate webView:didFinishLoadForFrame:]):
     15        (TestWebKitAPI):
     16        (TestWebKitAPI::TEST):
     17
    1182012-07-05  Dave Tharp  <dtharp@codeaurora.org>
    219
  • trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r121521 r121929  
    2020                1ADBEFE3130C6AA100D61D19 /* simple-accelerated-compositing.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 1ADBEFBC130C6A0100D61D19 /* simple-accelerated-compositing.html */; };
    2121                1AEDE22613E5E7E700E62FE8 /* InjectedBundleControllerMac.mm in Sources */ = {isa = PBXBuildFile; fileRef = 1AEDE22413E5E7A000E62FE8 /* InjectedBundleControllerMac.mm */; };
     22                26DF5A5E15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm in Sources */ = {isa = PBXBuildFile; fileRef = 26DF5A5D15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm */; };
     23                26DF5A6315A2A27E003689C2 /* CancelLoadFromResourceLoadDelegate.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 26DF5A6115A2A22B003689C2 /* CancelLoadFromResourceLoadDelegate.html */; };
    2224                333B9CE21277F23100FEFCE3 /* PreventEmptyUserAgent.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 333B9CE11277F23100FEFCE3 /* PreventEmptyUserAgent.cpp */; };
    2325                33BE5AF5137B5A6C00705813 /* MouseMoveAfterCrash.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 33BE5AF4137B5A6C00705813 /* MouseMoveAfterCrash.cpp */; };
     
    190192                                B55F11B71517D03300915916 /* attributedStringCustomFont.html in Copy Resources */,
    191193                                76E182DF154767E600F1FADD /* auto-submitting-form.html in Copy Resources */,
     194                                26DF5A6315A2A27E003689C2 /* CancelLoadFromResourceLoadDelegate.html in Copy Resources */,
    192195                                5142B2731517C8C800C32B19 /* ContextMenuCanCopyURL.html in Copy Resources */,
    193196                                9B4F8FA7159D52DD002D9F94 /* HTMLCollectionNamedItem.html in Copy Resources */,
     
    232235                1ADBEFBC130C6A0100D61D19 /* simple-accelerated-compositing.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "simple-accelerated-compositing.html"; sourceTree = "<group>"; };
    233236                1AEDE22413E5E7A000E62FE8 /* InjectedBundleControllerMac.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = InjectedBundleControllerMac.mm; sourceTree = "<group>"; };
     237                26DF5A5D15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CancelLoadFromResourceLoadDelegate.mm; sourceTree = "<group>"; };
     238                26DF5A6115A2A22B003689C2 /* CancelLoadFromResourceLoadDelegate.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = CancelLoadFromResourceLoadDelegate.html; sourceTree = "<group>"; };
    234239                333B9CE11277F23100FEFCE3 /* PreventEmptyUserAgent.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PreventEmptyUserAgent.cpp; sourceTree = "<group>"; };
    235240                33BE5AF4137B5A6C00705813 /* MouseMoveAfterCrash.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MouseMoveAfterCrash.cpp; sourceTree = "<group>"; };
     
    664669                                379028B514FABD92007E6B43 /* AcceptsFirstMouse.mm */,
    665670                                B55F119F1516834F00915916 /* AttributedString.mm */,
     671                                26DF5A5D15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm */,
    666672                                5142B2701517C88B00C32B19 /* ContextMenuCanCopyURL.mm */,
    667673                                37DC678B140D7C5000ABCCDB /* DOMRangeOfString.mm */,
     
    692698                                B55F11B01517A2C400915916 /* attributedStringCustomFont.html */,
    693699                                379028B814FABE49007E6B43 /* acceptsFirstMouse.html */,
     700                                26DF5A6115A2A22B003689C2 /* CancelLoadFromResourceLoadDelegate.html */,
    694701                                5142B2721517C89100C32B19 /* ContextMenuCanCopyURL.html */,
    695702                                37DC678F140D7D3A00ABCCDB /* DOMRangeOfString.html */,
     
    908915                                9B26FC6C159D061000CC3765 /* HTMLFormCollectionNamedItem.mm in Sources */,
    909916                                9B4F8FA4159D52B1002D9F94 /* HTMLCollectionNamedItem.mm in Sources */,
     917                                26DF5A5E15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm in Sources */,
    910918                        );
    911919                        runOnlyForDeploymentPostprocessing = 0;
Note: See TracChangeset for help on using the changeset viewer.