Changeset 57520 in webkit

Timestamp:

Apr 13, 2010 10:08:32 AM (14 years ago)

Author:

eric@webkit.org

Message:

2010-04-13 Jeremy Moskovich <​jeremy@chromium.org>

Reviewed by David Levin.

Add some diagnostics to try to track down cause of crash in ArchiveFactory::isArchiveMimeType().

​https://bugs.webkit.org/show_bug.cgi?id=36426

No new tests as there is no new functionality.

• loader/FrameLoader.cpp: (WebCore::FrameLoader::finishedLoadingDocument): Make copy of mimeType string to isolate crash.

2010-04-13 Jeremy Moskovich <​jeremy@chromium.org>

Reviewed by David Levin.

Add some diagnostics to try to track down cause of crash in ArchiveFactory::isArchiveMimeType().

​https://bugs.webkit.org/show_bug.cgi?id=36426

• src/ResourceHandle.cpp: Track state across ResourceHandle invocations. (WebCore::ResourceHandleInternal::ResourceHandleInternal): (WebCore::ResourceHandleInternal::): (WebCore::ResourceHandleInternal::start): (WebCore::ResourceHandleInternal::cancel): (WebCore::ResourceHandleInternal::didReceiveResponse): (WebCore::ResourceHandleInternal::didReceiveData): (WebCore::ResourceHandleInternal::didFinishLoading): (WebCore::ResourceHandleInternal::didFail):

Location:

trunk

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/loader/FrameLoader.cpp (modified) (1 diff)
• WebKit/chromium/ChangeLog (modified) (1 diff)
• WebKit/chromium/src/ResourceHandle.cpp (modified) (7 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-04-13  Jeremy Moskovich  <jeremy@chromium.org>
+
+        Reviewed by David Levin.
+
+        Add some diagnostics to try to track down cause of crash in ArchiveFactory::isArchiveMimeType().
+
+        https://bugs.webkit.org/show_bug.cgi?id=36426
+
+        No new tests as there is no new functionality.
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::finishedLoadingDocument): Make copy of mimeType string to isolate crash.
+
 2010-04-13  Abhishek Arya  <inferno@chromium.org>
 

trunk/WebCore/loader/FrameLoader.cpp

@@ -2871 +2871 @@
     
     // If loading a webarchive, run through webarchive machinery
+#if PLATFORM(CHROMIUM)
+    // https://bugs.webkit.org/show_bug.cgi?id=36426
+    // FIXME: For debugging purposes, should be removed before closing the bug.
+    // Make real copy of the string so we fail here if the responseMIMEType
+    // string is bad.
+    const String responseMIMEType = loader->responseMIMEType();
+#else
     const String& responseMIMEType = loader->responseMIMEType();
+#endif
 
     // FIXME: Mac's FrameLoaderClient::finishedLoading() method does work that is required even with Archive loads

trunk/WebKit/chromium/ChangeLog

@@ +1 @@
+2010-04-13  Jeremy Moskovich  <jeremy@chromium.org>
+
+        Reviewed by David Levin.
+
+        Add some diagnostics to try to track down cause of crash in ArchiveFactory::isArchiveMimeType().
+
+        https://bugs.webkit.org/show_bug.cgi?id=36426
+
+        * src/ResourceHandle.cpp: Track state across ResourceHandle invocations.
+        (WebCore::ResourceHandleInternal::ResourceHandleInternal):
+        (WebCore::ResourceHandleInternal::):
+        (WebCore::ResourceHandleInternal::start):
+        (WebCore::ResourceHandleInternal::cancel):
+        (WebCore::ResourceHandleInternal::didReceiveResponse):
+        (WebCore::ResourceHandleInternal::didReceiveData):
+        (WebCore::ResourceHandleInternal::didFinishLoading):
+        (WebCore::ResourceHandleInternal::didFail):
+
 2010-04-13  Mikhail Naganov  <mnaganov@chromium.org>
 

trunk/WebKit/chromium/src/ResourceHandle.cpp

@@ -58 +58 @@
         , m_owner(0)
         , m_client(client)
+        , m_state(ConnectionStateNew)
     {
     }
@@ -75 +76 @@
     virtual void didFail(WebURLLoader*, const WebURLError&);
 
+    enum ConnectionState {
+        ConnectionStateNew,
+        ConnectionStateStarted,
+        ConnectionStateReceivedResponse,
+        ConnectionStateReceivingData,
+        ConnectionStateFinishedLoading,
+        ConnectionStateCanceled,
+        ConnectionStateFailed,
+    };
+
     ResourceRequest m_request;
     ResourceHandle* m_owner;
     ResourceHandleClient* m_client;
     OwnPtr<WebURLLoader> m_loader;
+
+    // Used for sanity checking to make sure we don't experience illegal state
+    // transitions.
+    ConnectionState m_state;
 };
 
 void ResourceHandleInternal::start()
 {
+    if (m_state != ConnectionStateNew)
+        CRASH();
+    m_state = ConnectionStateStarted;
+
     m_loader.set(webKitClient()->createURLLoader());
     ASSERT(m_loader.get());
@@ -93 +112 @@
 void ResourceHandleInternal::cancel()
 {
+    m_state = ConnectionStateCanceled;
     m_loader->cancel();
 
@@ -129 +149 @@
     ASSERT(m_client);
     ASSERT(!response.isNull());
+    bool isMultipart = response.isMultipartPayload();
+    bool isValidStateTransition = (m_state == ConnectionStateStarted || m_state == ConnectionStateReceivedResponse);
+    // In the case of multipart loads, calls to didReceiveData & didReceiveResponse can be interleaved.
+    if (!isMultipart && !isValidStateTransition)
+        CRASH();
+    m_state = ConnectionStateReceivedResponse;
     m_client->didReceiveResponse(m_owner, response.toResourceResponse());
 }
@@ -136 +162 @@
 {
     ASSERT(m_client);
+    if (m_state != ConnectionStateReceivedResponse && m_state != ConnectionStateReceivingData)
+        CRASH();
+    m_state = ConnectionStateReceivingData;
 
     // FIXME(yurys): it looks like lengthReceived is always the same as
@@ -146 +175 @@
 {
     ASSERT(m_client);
+    if (m_state != ConnectionStateReceivedResponse && m_state != ConnectionStateReceivingData)
+        CRASH();
+    m_state = ConnectionStateFinishedLoading;
     m_client->didFinishLoading(m_owner);
 }
@@ -152 +184 @@
 {
     ASSERT(m_client);
+    m_state = ConnectionStateFailed;
     m_client->didFail(m_owner, error);
 }