Changeset 144652 in webkit

Timestamp:

Mar 4, 2013 11:32:03 AM (11 years ago)

Author:

commit-queue@webkit.org

Message:

[GTK] Some WebKit2 GTK+ unit tests are failing in 32 bits bot
​https://bugs.webkit.org/show_bug.cgi?id=111346

Patch by Carlos Garcia Campos <​cgarcia@igalia.com> on 2013-03-04
Reviewed by Martin Robinson.

The problem is the use of a temporary CString object in the macro
g_assert_cmpstr. It's a common mistake because we usually forget
that g_assert_cmpstr is not a function but a macro, that expands
to multiple lines. This patch adds a new macro ASSERT_CMP_CSTRING
with the same implementation that g_assert_cmpstr, but using
CStrings instead of const char*. It fixes all the cases where a
temporary CString was used in g_assert_cmpstr, and uses the new
macro also for the cases where we were caching the CString just
for g_assert_cmpstr.

• UIProcess/API/gtk/tests/TestDownloads.cpp:

(testDownloadRemoteFile):

• UIProcess/API/gtk/tests/TestInspectorServer.cpp:

(testInspectorServerPageList):

• UIProcess/API/gtk/tests/TestLoaderClient.cpp:
• UIProcess/API/gtk/tests/TestMain.h:
• UIProcess/API/gtk/tests/TestResources.cpp:
• UIProcess/API/gtk/tests/TestWebKitFaviconDatabase.cpp:

(testGetFaviconURI):

• UIProcess/API/gtk/tests/TestWebKitSettings.cpp:

(assertThatUserAgentIsSentInHeaders):
(testWebKitSettingsUserAgent):

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/API/gtk/tests/TestDownloads.cpp (modified) (1 diff)
• UIProcess/API/gtk/tests/TestInspectorServer.cpp (modified) (1 diff)
• UIProcess/API/gtk/tests/TestLoaderClient.cpp (modified) (1 diff)
• UIProcess/API/gtk/tests/TestMain.h (modified) (1 diff)
• UIProcess/API/gtk/tests/TestResources.cpp (modified) (1 diff)
• UIProcess/API/gtk/tests/TestWebKitFaviconDatabase.cpp (modified) (1 diff)
• UIProcess/API/gtk/tests/TestWebKitSettings.cpp (modified) (2 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2013-03-04  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] Some WebKit2 GTK+ unit tests are failing in 32 bits bot
+        https://bugs.webkit.org/show_bug.cgi?id=111346
+
+        Reviewed by Martin Robinson.
+
+        The problem is the use of a temporary CString object in the macro
+        g_assert_cmpstr. It's a common mistake because we usually forget
+        that g_assert_cmpstr is not a function but a macro, that expands
+        to multiple lines. This patch adds a new macro ASSERT_CMP_CSTRING
+        with the same implementation that g_assert_cmpstr, but using
+        CStrings instead of const char*. It fixes all the cases where a
+        temporary CString was used in g_assert_cmpstr, and uses the new
+        macro also for the cases where we were caching the CString just
+        for g_assert_cmpstr.
+
+        * UIProcess/API/gtk/tests/TestDownloads.cpp:
+        (testDownloadRemoteFile):
+        * UIProcess/API/gtk/tests/TestInspectorServer.cpp:
+        (testInspectorServerPageList):
+        * UIProcess/API/gtk/tests/TestLoaderClient.cpp:
+        * UIProcess/API/gtk/tests/TestMain.h:
+        * UIProcess/API/gtk/tests/TestResources.cpp:
+        * UIProcess/API/gtk/tests/TestWebKitFaviconDatabase.cpp:
+        (testGetFaviconURI):
+        * UIProcess/API/gtk/tests/TestWebKitSettings.cpp:
+        (assertThatUserAgentIsSentInHeaders):
+        (testWebKitSettingsUserAgent):
+
 2013-03-04  Kunihiko Sakamoto  <ksakamoto@chromium.org>
 

trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestDownloads.cpp

@@ -342 +342 @@
     WebKitURIRequest* request = webkit_download_get_request(download.get());
     g_assert(request);
-    CString requestURI = kServer->getURIForPath("/test.pdf");
-    g_assert_cmpstr(webkit_uri_request_get_uri(request), ==, requestURI.data());
+    ASSERT_CMP_CSTRING(webkit_uri_request_get_uri(request), ==, kServer->getURIForPath("/test.pdf"));
 
     g_assert(webkit_download_get_destination(download.get()));

trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp

@@ -191 +191 @@
     valueString.set(WebViewTest::javascriptResultToCString(javascriptResult));
     String validInspectorURL = String("/webinspector/inspector.html?page=") + String::number(pageId);
-    g_assert_cmpstr(valueString.get(), ==, validInspectorURL.utf8().data());
+    ASSERT_CMP_CSTRING(valueString.get(), ==, validInspectorURL.utf8());
 }
 

trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestLoaderClient.cpp

@@ -215 +215 @@
     void checkActiveURI(const char* uri)
     {
-        // g_assert_cmpstr is a macro, so we need to cache the temporary string.
-        CString serverURI = kServer->getURIForPath(uri);
-        g_assert_cmpstr(m_activeURI.data(), ==, serverURI.data());
+        ASSERT_CMP_CSTRING(m_activeURI, ==, kServer->getURIForPath(uri));
     }
 };

trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestMain.h

@@ -40 +40 @@
         g_test_add(testPath.get(), ClassName, 0, ClassName::setUp, testFunc, ClassName::tearDown); \
     }
+
+#define ASSERT_CMP_CSTRING(s1, cmp, s2) \
+    do { CString __s1 = (s1); CString __s2 = (s2); \
+        if (g_strcmp0(__s1.data(), __s2.data()) cmp 0) ; else \
+            g_assertion_message_cmpstr(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \
+                #s1 " " #cmp " " #s2, __s1.data(), #cmp, __s2.data()); } while (0)
 
 class Test {

trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestResources.cpp

@@ -504 +504 @@
     void checkActiveURI(const char* uri)
     {
-        // g_assert_cmpstr is a macro, so we need to cache the temporary string.
-        CString serverURI = kServer->getURIForPath(uri);
-        g_assert_cmpstr(m_activeURI.data(), ==, serverURI.data());
+        ASSERT_CMP_CSTRING(m_activeURI, ==, kServer->getURIForPath(uri));
     }
 };

trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitFaviconDatabase.cpp

@@ -203 +203 @@
     WebKitFaviconDatabase* database = webkit_web_context_get_favicon_database(test->m_webContext);
 
-    const char* baseURI = kServer->getURIForPath("/foo").data();
-    GOwnPtr<char> iconURI(webkit_favicon_database_get_favicon_uri(database, baseURI));
-    g_assert_cmpstr(iconURI.get(), ==, kServer->getURIForPath("/icon/favicon.ico").data());
+    CString baseURI = kServer->getURIForPath("/foo");
+    GOwnPtr<char> iconURI(webkit_favicon_database_get_favicon_uri(database, baseURI.data()));
+    ASSERT_CMP_CSTRING(iconURI.get(), ==, kServer->getURIForPath("/icon/favicon.ico"));
 }
 

trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp

@@ -275 +275 @@
     test->loadURI(gServer->getURIForPath("/").data());
     test->waitUntilLoadFinished();
-    g_assert_cmpstr(convertWebViewMainResourceDataToCString(test).data(), ==, userAgent.data());
+    ASSERT_CMP_CSTRING(convertWebViewMainResourceDataToCString(test), ==, userAgent);
 }
 
@@ -301 +301 @@
 
     webkit_settings_set_user_agent_with_application_details(settings.get(), "WebKitGTK+", 0);
-    CString userAgentWithNullVersion = webkit_settings_get_user_agent(settings.get());
-    g_assert_cmpstr(g_strstr_len(userAgentWithNullVersion.data(), -1, defaultUserAgent.data()), ==, userAgentWithNullVersion.data());
-    g_assert(g_strstr_len(userAgentWithNullVersion.data(), -1, "WebKitGTK+"));
+    const char* userAgentWithNullVersion = webkit_settings_get_user_agent(settings.get());
+    g_assert_cmpstr(g_strstr_len(userAgentWithNullVersion, -1, defaultUserAgent.data()), ==, userAgentWithNullVersion);
+    g_assert(g_strstr_len(userAgentWithNullVersion, -1, "WebKitGTK+"));
 
     webkit_settings_set_user_agent_with_application_details(settings.get(), "WebKitGTK+", "");
-    g_assert_cmpstr(webkit_settings_get_user_agent(settings.get()), ==, userAgentWithNullVersion.data());
+    g_assert_cmpstr(webkit_settings_get_user_agent(settings.get()), ==, userAgentWithNullVersion);
 
     webkit_settings_set_user_agent_with_application_details(settings.get(), "WebCatGTK+", "3.4.5");