Changeset 144055 in webkit

Timestamp:

Feb 26, 2013 7:43:07 AM (11 years ago)

Author:

commit-queue@webkit.org

Message:

REGRESSION (r143619): Crashes in three layout tests
​https://bugs.webkit.org/show_bug.cgi?id=110588

Patch by Martin Robinson <​mrobinson@igalia.com> on 2013-02-26
Reviewed by Gustavo Noronha Silva.

Source/WebKit/gtk:

Guard against null main resource identifiers. The main resource
identifier can be null at various times during the load. A null
identifier is never equal to the ones we are looking to remove.

• WebCoreSupport/FrameLoaderClientGtk.cpp:

(WebKit::FrameLoaderClient::dispatchDidFinishLoading): Use the new webkitWebViewRemoveSubresource helper.
(WebKit::FrameLoaderClient::dispatchDidFailLoading): ditto.

• webkit/webkitwebview.cpp:

(webkitWebViewRemoveSubresource): Added this helper which removes a subresource, but
never touches the main resource. This is adapted from the old method, for which the
main resource branch was dead code.

• webkit/webkitwebviewprivate.h: Update the method list.

LayoutTests:

• platform/gtk/TestExpectations: Unskip some tests which are no longer

crashing.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/gtk/TestExpectations (modified) (1 diff)
• Source/WebKit/gtk/ChangeLog (modified) (1 diff)
• Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp (modified) (2 diffs)
• Source/WebKit/gtk/webkit/webkitwebview.cpp (modified) (1 diff)
• Source/WebKit/gtk/webkit/webkitwebviewprivate.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-02-26  Martin Robinson  <mrobinson@igalia.com>
+
+        REGRESSION (r143619): Crashes in three layout tests
+        https://bugs.webkit.org/show_bug.cgi?id=110588
+
+        Reviewed by Gustavo Noronha Silva.
+
+        * platform/gtk/TestExpectations: Unskip some tests which are no longer
+        crashing.
+
 2013-02-26  Andrey Kosyakov  <caseq@chromium.org>
 

trunk/LayoutTests/platform/gtk/TestExpectations

@@ -491 +491 @@
 
 webkit.org/b/110222 fast/events/platform-wheelevent-with-delta-zero-crash.html [ Crash ]
-
-webkit.org/b/110588 http/tests/misc/iframe-reparenting-id-collision.html [ Crash Pass ]
-webkit.org/b/110588 http/tests/misc/window-open-then-write.html [ Crash ]
-webkit.org/b/110588 http/tests/xmlhttprequest/request-from-popup.html [ Crash ]
 
 webkit.org/b/110695 http/tests/security/cross-origin-local-storage-allowed.html [ Crash Pass ]

trunk/Source/WebKit/gtk/ChangeLog

@@ +1 @@
+2013-02-26  Martin Robinson  <mrobinson@igalia.com>
+
+        REGRESSION (r143619): Crashes in three layout tests
+        https://bugs.webkit.org/show_bug.cgi?id=110588
+
+        Reviewed by Gustavo Noronha Silva.
+
+        Guard against null main resource identifiers. The main resource
+        identifier can be null at various times during the load. A null
+        identifier is never equal to the ones we are looking to remove.
+
+        * WebCoreSupport/FrameLoaderClientGtk.cpp:
+        (WebKit::FrameLoaderClient::dispatchDidFinishLoading): Use the new webkitWebViewRemoveSubresource helper.
+        (WebKit::FrameLoaderClient::dispatchDidFailLoading): ditto.
+        * webkit/webkitwebview.cpp:
+        (webkitWebViewRemoveSubresource): Added this helper which removes a subresource, but
+        never touches the main resource. This is adapted from the old method, for which the
+        main resource branch was dead code.
+        * webkit/webkitwebviewprivate.h: Update the method list.
+
 2013-02-25  Andreas Kling  <akling@apple.com>
 

trunk/Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp

@@ -1022 +1022 @@
     g_signal_emit_by_name(webView, "resource-load-finished", m_frame, webResource);
 
-    if (!g_str_equal(identifierString.get(), webView->priv->mainResourceIdentifier.data()))
-        webkit_web_view_remove_resource(webView, identifierString.get());
+    webkitWebViewRemoveSubresource(webView, identifierString.get());
 }
 
@@ -1047 +1046 @@
     g_signal_emit_by_name(webView, "resource-load-failed", m_frame, webResource, webError.get());
 
-    if (!g_str_equal(identifierString.get(), webView->priv->mainResourceIdentifier.data()))
-        webkit_web_view_remove_resource(webView, identifierString.get());
+    webkitWebViewRemoveSubresource(webView, identifierString.get());
 }
 

trunk/Source/WebKit/gtk/webkit/webkitwebview.cpp

@@ -5067 +5067 @@
 }
 
-void webkit_web_view_remove_resource(WebKitWebView* webView, const char* identifier)
-{
-    WebKitWebViewPrivate* priv = webView->priv;
-    if (g_str_equal(identifier, priv->mainResourceIdentifier.data())) {
-        priv->mainResourceIdentifier = "";
-        priv->mainResource = 0;
-    } else
-      g_hash_table_remove(priv->subResources.get(), identifier);
+void webkitWebViewRemoveSubresource(WebKitWebView* webView, const char* identifier)
+{
+    ASSERT(identifier);
+
+    // Don't remove the main resource.
+    const CString& mainResource = webView->priv->mainResourceIdentifier;
+    if (!mainResource.isNull() && g_str_equal(identifier, mainResource.data()))
+        return;
+    g_hash_table_remove(webView->priv->subResources.get(), identifier);
 }
 

trunk/Source/WebKit/gtk/webkit/webkitwebviewprivate.h

@@ -130 +130 @@
 void webkit_web_view_add_resource(WebKitWebView*, const char*, WebKitWebResource*);
 void webkit_web_view_add_main_resource(WebKitWebView*, const char*, WebKitWebResource*);
-void webkit_web_view_remove_resource(WebKitWebView*, const char*);
+void webkitWebViewRemoveSubresource(WebKitWebView*, const char*);
 WebKitWebResource* webkit_web_view_get_resource(WebKitWebView*, char*);
 WebKitWebResource* webkit_web_view_get_main_resource(WebKitWebView*);