Changeset 56453 in webkit

Timestamp:

Mar 24, 2010 1:20:32 PM (14 years ago)

Author:

eric@webkit.org

Message:

2010-03-24 Jeremy Moskovich <​jeremy@chromium.org>

Reviewed by Jeremy Orlow.

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-03-24 Jeremy Moskovich <​jeremy@chromium.org>

Reviewed by Jeremy Orlow.

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-03-24  Jeremy Moskovich  <jeremy@chromium.org>
+
+        Reviewed by Jeremy Orlow.
+
+        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-03-24  Anton Muhin  <antonm@chromium.org>
 

trunk/WebCore/loader/FrameLoader.cpp

@@ -2813 +2813 @@
     
     // If loading a webarchive, run through webarchive machinery
+#if PLATFORM(CHROMIUM)
+    // https://bugs.webkit.org/show_bug.cgi?id=36426
+    // FIXME: jeremy@chromium.org - 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-03-24  Jeremy Moskovich  <jeremy@chromium.org>
+
+        Reviewed by Jeremy Orlow.
+
+        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-03-24  Jay Campan  <jcampan@google.com>
 

trunk/WebKit/chromium/src/ResourceHandle.cpp

@@ -58 +58 @@
         , m_owner(0)
         , m_client(client)
+        , m_state(CONNECTION_STATE_NEW)
     {
     }
@@ -75 +76 @@
     virtual void didFail(WebURLLoader*, const WebURLError&);
 
+    enum ConnectionState {
+        CONNECTION_STATE_NEW,
+        CONNECTION_STATE_STARTED,
+        CONNECTION_STATE_RECEIVED_RESPONSE,
+        CONNECTION_STATE_RECEIVING_DATA,
+        CONNECTION_STATE_FINISHED_LOADING,
+        CONNECTION_STATE_CANCELED,
+        CONNECTION_STATE_FAILED,
+    };
+
     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 == CONNECTION_STATE_NEW))
+        CRASH();
+    m_state = CONNECTION_STATE_STARTED;
+
     m_loader.set(webKitClient()->createURLLoader());
     ASSERT(m_loader.get());
@@ -93 +112 @@
 void ResourceHandleInternal::cancel()
 {
+    m_state = CONNECTION_STATE_CANCELED;
     m_loader->cancel();
 
@@ -129 +149 @@
     ASSERT(m_client);
     ASSERT(!response.isNull());
+    if (!(m_state == CONNECTION_STATE_STARTED))
+        CRASH();
+    m_state = CONNECTION_STATE_RECEIVED_RESPONSE;
     m_client->didReceiveResponse(m_owner, response.toResourceResponse());
 }
@@ -136 +159 @@
 {
     ASSERT(m_client);
+    if (!(m_state == CONNECTION_STATE_RECEIVED_RESPONSE
+          || m_state == CONNECTION_STATE_RECEIVING_DATA))
+        CRASH();
+    m_state = CONNECTION_STATE_RECEIVING_DATA;
 
     // FIXME(yurys): it looks like lengthReceived is always the same as
@@ -146 +173 @@
 {
     ASSERT(m_client);
+    if (!(m_state == CONNECTION_STATE_RECEIVED_RESPONSE
+          || m_state == CONNECTION_STATE_RECEIVING_DATA))
+        CRASH();
+    m_state = CONNECTION_STATE_FINISHED_LOADING;
     m_client->didFinishLoading(m_owner);
 }
@@ -152 +183 @@
 {
     ASSERT(m_client);
+    m_state = CONNECTION_STATE_FAILED;
     m_client->didFail(m_owner, error);
 }