From 8e5df79d7272a36e7c431e99ebf5d79d19ecd82c Mon Sep 17 00:00:00 2001
From: "sergio@webkit.org"
 <sergio@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed, 28 Aug 2013 09:29:30 +0000
Subject: [PATCH] [Soup] WebTiming information not shown in the inspector
 https://bugs.webkit.org/show_bug.cgi?id=118395

Reviewed by Martin Robinson.

Source/WebCore:

WebTiming information was not correctly provided to WebCore
because the gotHeadersCallback was incorrectly resetting the
original ResourceResponse (which had the ResourceLoadTiming
object) instead of simply updating their contents using the
SoupMessage.

No new test required as this feature is already covered by the
existing webtiming tests. In any case this change includes a fix
for the http/tests/misc/webtiming-ssl.php test which was not
failing even if it should because it was not correct.

* platform/network/soup/ResourceHandleSoup.cpp:
(WebCore::gotHeadersCallback):
(WebCore::restartedCallback): restartedCallback should be
available only if WEB_TIMING is defined.
(WebCore::createSoupMessageForHandleAndRequest): Ditto.

LayoutTests:

Fixed the webiming-ssl.php test that was incorrectly considering a
FAIL as the expected result. It was probably just a legacy
decision which came from the times where DRT was responsible of
reporting WebTiming information.

Also added a new test that fails due to wkb.ug/103927. It was not
detected before because there were no timing information in the
response.

* http/tests/misc/resources/webtiming-ssl.html:
* http/tests/misc/webtiming-ssl-expected.txt:
* platform/gtk/TestExpectations: added
http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_attribute_order.html.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@154727 268f45cc-cd09-0410-ab3c-d52691b4dbfc
---
 LayoutTests/ChangeLog                         | 21 ++++++++++++++++
 .../tests/misc/resources/webtiming-ssl.html   |  2 +-
 .../tests/misc/webtiming-ssl-expected.txt     |  4 ++--
 LayoutTests/platform/gtk/TestExpectations     |  1 +
 Source/WebCore/ChangeLog                      | 24 +++++++++++++++++++
 .../network/soup/ResourceHandleSoup.cpp       | 10 ++++----
 6 files changed, 53 insertions(+), 9 deletions(-)

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index a908c113568..1d189e9264b 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,24 @@
+2013-08-28  Sergio Villar Senin  <svillar@igalia.com>
+
+        [Soup] WebTiming information not shown in the inspector
+        https://bugs.webkit.org/show_bug.cgi?id=118395
+
+        Reviewed by Martin Robinson.
+
+        Fixed the webiming-ssl.php test that was incorrectly considering a
+        FAIL as the expected result. It was probably just a legacy
+        decision which came from the times where DRT was responsible of
+        reporting WebTiming information.
+
+        Also added a new test that fails due to wkb.ug/103927. It was not
+        detected before because there were no timing information in the
+        response.
+
+        * http/tests/misc/resources/webtiming-ssl.html:
+        * http/tests/misc/webtiming-ssl-expected.txt:
+        * platform/gtk/TestExpectations: added
+        http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_attribute_order.html.
+
 2013-08-28  Gabor Rapcsanyi  <rgabor@webkit.org>
 
         Unreviewed ARM Qt gardening.
diff --git a/LayoutTests/http/tests/misc/resources/webtiming-ssl.html b/LayoutTests/http/tests/misc/resources/webtiming-ssl.html
index 5848e2f0fec..d9607934be6 100644
--- a/LayoutTests/http/tests/misc/resources/webtiming-ssl.html
+++ b/LayoutTests/http/tests/misc/resources/webtiming-ssl.html
@@ -9,7 +9,7 @@
 <p id="description"></p>
 <div id="console"></div>
 <script>
-description("This test checks that Web Timing reports secureConnectionStart correctly and has reasonable values for connectStart and connectEnd. Note that DumpRenderTree doesn't set secureConnectionStart.");
+description("This test checks that Web Timing reports secureConnectionStart correctly and has reasonable values for connectStart and connectEnd.");
 
 window.performance = window.performance || {};
 var navigation = performance.navigation || {};
diff --git a/LayoutTests/http/tests/misc/webtiming-ssl-expected.txt b/LayoutTests/http/tests/misc/webtiming-ssl-expected.txt
index 4dc4c2ec362..a91f59b2b7d 100644
--- a/LayoutTests/http/tests/misc/webtiming-ssl-expected.txt
+++ b/LayoutTests/http/tests/misc/webtiming-ssl-expected.txt
@@ -1,10 +1,10 @@
-This test checks that Web Timing reports secureConnectionStart correctly and has reasonable values for connectStart and connectEnd. Note that DumpRenderTree doesn't set secureConnectionStart.
+This test checks that Web Timing reports secureConnectionStart correctly and has reasonable values for connectStart and connectEnd.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
 PASS timing.connectStart is >= timing.navigationStart
-FAIL timing.secureConnectionStart should be >= timing.connectStart. Was 0 (of type number).
+PASS timing.secureConnectionStart is >= timing.connectStart
 PASS timing.connectEnd is >= timing.secureConnectionStart
 PASS successfullyParsed is true
 
diff --git a/LayoutTests/platform/gtk/TestExpectations b/LayoutTests/platform/gtk/TestExpectations
index 9fffb7f9640..7349e1355b2 100644
--- a/LayoutTests/platform/gtk/TestExpectations
+++ b/LayoutTests/platform/gtk/TestExpectations
@@ -1274,6 +1274,7 @@ webkit.org/b/106924 accessibility/heading-level.html [ Failure ]
 
 webkit.org/b/103927 http/tests/w3c/webperf/submission/Intel/resource-timing/test_resource_timing_cross_origin_resource_request.html [ Failure ]
 webkit.org/b/103927 http/tests/w3c/webperf/submission/Intel/resource-timing/test_resource_timing_timing_allow_cross_origin_resource_request.html [ Failure ]
+webkit.org/b/103927 http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_attribute_order.html [ Failure ]
 webkit.org/b/103927 http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_frame_initiator_type.html [ Failure ]
 webkit.org/b/103927 http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_initiator_types.html [ Failure ]
 webkit.org/b/103927 http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_redirects.html [ Failure ]
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index bce93d489d9..4671dc0d9a9 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2013-08-28  Sergio Villar Senin  <svillar@igalia.com>
+
+        [Soup] WebTiming information not shown in the inspector
+        https://bugs.webkit.org/show_bug.cgi?id=118395
+
+        Reviewed by Martin Robinson.
+
+        WebTiming information was not correctly provided to WebCore
+        because the gotHeadersCallback was incorrectly resetting the
+        original ResourceResponse (which had the ResourceLoadTiming
+        object) instead of simply updating their contents using the
+        SoupMessage.
+
+        No new test required as this feature is already covered by the
+        existing webtiming tests. In any case this change includes a fix
+        for the http/tests/misc/webtiming-ssl.php test which was not
+        failing even if it should because it was not correct.
+
+        * platform/network/soup/ResourceHandleSoup.cpp:
+        (WebCore::gotHeadersCallback):
+        (WebCore::restartedCallback): restartedCallback should be
+        available only if WEB_TIMING is defined.
+        (WebCore::createSoupMessageForHandleAndRequest): Ditto.
+
 2013-08-26  Antonio Gomes  <a1.gomes@sisa.samsung.com>
 
         Scrolling allowed when overflow:hidden (seen on Acid2)
diff --git a/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp b/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp
index 9e704a26380..f660e904e55 100644
--- a/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp
+++ b/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp
@@ -352,9 +352,7 @@ static void gotHeadersCallback(SoupMessage* message, gpointer data)
 
     // The original response will be needed later to feed to willSendRequest in
     // doRedirect() in case we are redirected. For this reason, we store it here.
-    ResourceResponse response;
-    response.updateFromSoupMessage(message);
-    d->m_response = response;
+    d->m_response.updateFromSoupMessage(message);
 }
 
 static void applyAuthenticationToRequest(ResourceHandle* handle, ResourceRequest& request, bool redirect)
@@ -393,6 +391,7 @@ static void applyAuthenticationToRequest(ResourceHandle* handle, ResourceRequest
     request.setURL(urlWithCredentials);
 }
 
+#if ENABLE(WEB_TIMING)
 // Called each time the message is going to be sent again except the first time.
 // This happens when libsoup handles HTTP authentication.
 static void restartedCallback(SoupMessage*, gpointer data)
@@ -401,13 +400,12 @@ static void restartedCallback(SoupMessage*, gpointer data)
     if (!handle || handle->cancelledOrClientless())
         return;
 
-#if ENABLE(WEB_TIMING)
     ResourceHandleInternal* d = handle->getInternal();
     ResourceResponse& redirectResponse = d->m_response;
     redirectResponse.setResourceLoadTiming(ResourceLoadTiming::create());
     redirectResponse.resourceLoadTiming()->requestTime = monotonicallyIncreasingTime();
-#endif
 }
+#endif
 
 static bool shouldRedirect(ResourceHandle* handle)
 {
@@ -970,7 +968,6 @@ static bool createSoupMessageForHandleAndRequest(ResourceHandle* handle, const R
         soup_message_headers_set_content_length(soupMessage->request_headers, 0);
 
     g_signal_connect(d->m_soupMessage.get(), "got-headers", G_CALLBACK(gotHeadersCallback), handle);
-    g_signal_connect(d->m_soupMessage.get(), "restarted", G_CALLBACK(restartedCallback), handle);
     g_signal_connect(d->m_soupMessage.get(), "wrote-body-data", G_CALLBACK(wroteBodyDataCallback), handle);
 
     soup_message_set_flags(d->m_soupMessage.get(), static_cast<SoupMessageFlags>(soup_message_get_flags(d->m_soupMessage.get()) | SOUP_MESSAGE_NO_REDIRECT));
@@ -978,6 +975,7 @@ static bool createSoupMessageForHandleAndRequest(ResourceHandle* handle, const R
 #if ENABLE(WEB_TIMING)
     d->m_response.setResourceLoadTiming(ResourceLoadTiming::create());
     g_signal_connect(d->m_soupMessage.get(), "network-event", G_CALLBACK(networkEventCallback), handle);
+    g_signal_connect(d->m_soupMessage.get(), "restarted", G_CALLBACK(restartedCallback), handle);
     g_signal_connect(d->m_soupMessage.get(), "wrote-body", G_CALLBACK(wroteBodyCallback), handle);
 #endif
 
-- 
GitLab