Web Inspector: XHR with text but responseType = "blob" shows blank content
authorjoepeck@webkit.org <joepeck@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Aug 2016 22:16:23 +0000 (22:16 +0000)
committerjoepeck@webkit.org <joepeck@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Aug 2016 22:16:23 +0000 (22:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=161422
<rdar://problem/28066869>

Reviewed by Brian Burg.

Source/WebCore:

Test: inspector/network/xhr-json-blob-has-content.html

When an XMLHttpRequest finished loading it was always setting the Inspector's
content for that load at the end. However, if the XHR was loading binary data
then it was passing an empty string to the inspector and overwriting the
data the inspector already had for the resource. Avoid this overwriting.

* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::didFinishLoading):
When loading binary content we have no decoded text to send to the inspector.

* inspector/InspectorInstrumentation.cpp:
(WebCore::InspectorInstrumentation::didFinishXHRLoadingImpl):
* inspector/InspectorInstrumentation.h:
(WebCore::InspectorInstrumentation::didFinishXHRLoading):
Switch to an Optional string, and if it is not available don't
call through to the NetworkAgent expecting decoded text.

* inspector/InspectorNetworkAgent.cpp:
(WebCore::InspectorNetworkAgent::didFinishXHRLoading):
* inspector/InspectorNetworkAgent.h:
Improve variable name.

LayoutTests:

* inspector/network/resources/data.json: Added.
* inspector/network/xhr-json-blob-has-content-expected.txt: Added.
* inspector/network/xhr-json-blob-has-content.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@205268 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/inspector/network/resources/data.json [new file with mode: 0644]
LayoutTests/inspector/network/xhr-json-blob-has-content-expected.txt [new file with mode: 0644]
LayoutTests/inspector/network/xhr-json-blob-has-content.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/inspector/InspectorInstrumentation.cpp
Source/WebCore/inspector/InspectorInstrumentation.h
Source/WebCore/inspector/InspectorNetworkAgent.cpp
Source/WebCore/inspector/InspectorNetworkAgent.h
Source/WebCore/xml/XMLHttpRequest.cpp

index c3da998..f37f385 100644 (file)
@@ -1,3 +1,15 @@
+2016-08-31  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: XHR with text but responseType = "blob" shows blank content
+        https://bugs.webkit.org/show_bug.cgi?id=161422
+        <rdar://problem/28066869>
+
+        Reviewed by Brian Burg.
+
+        * inspector/network/resources/data.json: Added.
+        * inspector/network/xhr-json-blob-has-content-expected.txt: Added.
+        * inspector/network/xhr-json-blob-has-content.html: Added.
+
 2016-08-31  Ryosuke Niwa  <rniwa@webkit.org>
 
         HTML constructor must throw when newTarget is itself
diff --git a/LayoutTests/inspector/network/resources/data.json b/LayoutTests/inspector/network/resources/data.json
new file mode 100644 (file)
index 0000000..a89438e
--- /dev/null
@@ -0,0 +1 @@
+{"alpha":"beta","gamma":12345}
diff --git a/LayoutTests/inspector/network/xhr-json-blob-has-content-expected.txt b/LayoutTests/inspector/network/xhr-json-blob-has-content-expected.txt
new file mode 100644 (file)
index 0000000..a16cbf7
--- /dev/null
@@ -0,0 +1,14 @@
+Tests that an XMLHttpRequest resource gives us JSON text even if it is marked as having blob content.
+
+
+== Running test suite: XHR.Blob
+-- Running test case: XHR.JSONContent
+PASS: Resource should be created.
+PASS: Resource should complete loading.
+PASS: Resource has expected content.
+
+-- Running test case: XHR.JSONContent.Blob
+PASS: Resource should be created.
+PASS: Resource should complete loading.
+PASS: Resource has expected content.
+
diff --git a/LayoutTests/inspector/network/xhr-json-blob-has-content.html b/LayoutTests/inspector/network/xhr-json-blob-has-content.html
new file mode 100644 (file)
index 0000000..e407d91
--- /dev/null
@@ -0,0 +1,68 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
+<script>
+function createJSONXHR() {
+    let xhr = new XMLHttpRequest;
+    xhr.open("GET", "resources/data.json?" + Math.random(), true);
+    xhr.send();
+}
+
+function createJSONBlobXHR() {
+    let xhr = new XMLHttpRequest;
+    xhr.open("GET", "resources/data.json?" + Math.random(), true);
+    xhr.responseType = "blob";
+    xhr.send();
+}
+
+function test()
+{
+    let suite = InspectorTest.createAsyncSuite("XHR.Blob");
+
+    const jsonContent = `{"alpha":"beta","gamma":12345}\n`;
+
+    suite.addTestCase({
+        name: "XHR.JSONContent",
+        description: "Ensure an XMLHttpRequest with JSON content still gives us text.",
+        test: (resolve, reject) => {
+            InspectorTest.evaluateInPage("createJSONXHR()");
+            WebInspector.Frame.singleFireEventListener(WebInspector.Frame.Event.ResourceWasAdded, (event) => {
+                let resource = event.data.resource;
+                InspectorTest.expectThat(resource instanceof WebInspector.Resource, "Resource should be created.");
+                resource.singleFireEventListener(WebInspector.Resource.Event.LoadingDidFinish, (event) => {
+                    InspectorTest.pass("Resource should complete loading.");
+                    resource.requestContent().then(() => {
+                        InspectorTest.expectThat(resource.content === jsonContent, "Resource has expected content.");
+                    }).then(resolve, reject);
+                });
+            });
+        }
+    });
+
+    suite.addTestCase({
+        name: "XHR.JSONContent.Blob",
+        description: "Ensure an XMLHttpRequest with JSON content and a responseType of blob still gives us text.",
+        test: (resolve, reject) => {
+            InspectorTest.evaluateInPage("createJSONBlobXHR()");
+            WebInspector.Frame.singleFireEventListener(WebInspector.Frame.Event.ResourceWasAdded, (event) => {
+                let resource = event.data.resource;
+                InspectorTest.expectThat(resource instanceof WebInspector.Resource, "Resource should be created.");
+                resource.singleFireEventListener(WebInspector.Resource.Event.LoadingDidFinish, (event) => {
+                    InspectorTest.pass("Resource should complete loading.");
+                    resource.requestContent().then(() => {
+                        InspectorTest.expectThat(resource.content === jsonContent, "Resource has expected content.");
+                    }).then(resolve, reject);
+                });
+            });
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>Tests that an XMLHttpRequest resource gives us JSON text even if it is marked as having blob content.</p>
+</body>
+</html>
index 92b4441..547f3d3 100644 (file)
@@ -1,3 +1,34 @@
+2016-08-31  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: XHR with text but responseType = "blob" shows blank content
+        https://bugs.webkit.org/show_bug.cgi?id=161422
+        <rdar://problem/28066869>
+
+        Reviewed by Brian Burg.
+
+        Test: inspector/network/xhr-json-blob-has-content.html
+
+        When an XMLHttpRequest finished loading it was always setting the Inspector's
+        content for that load at the end. However, if the XHR was loading binary data
+        then it was passing an empty string to the inspector and overwriting the
+        data the inspector already had for the resource. Avoid this overwriting.
+
+        * xml/XMLHttpRequest.cpp:
+        (WebCore::XMLHttpRequest::didFinishLoading):
+        When loading binary content we have no decoded text to send to the inspector.
+
+        * inspector/InspectorInstrumentation.cpp:
+        (WebCore::InspectorInstrumentation::didFinishXHRLoadingImpl):
+        * inspector/InspectorInstrumentation.h:
+        (WebCore::InspectorInstrumentation::didFinishXHRLoading):
+        Switch to an Optional string, and if it is not available don't
+        call through to the NetworkAgent expecting decoded text.
+
+        * inspector/InspectorNetworkAgent.cpp:
+        (WebCore::InspectorNetworkAgent::didFinishXHRLoading):
+        * inspector/InspectorNetworkAgent.h:
+        Improve variable name.
+
 2016-08-31  Alex Christensen  <achristensen@webkit.org>
 
         Add runtime flag for using URLParser
index b96dd01..ffe571a 100644 (file)
@@ -635,12 +635,14 @@ void InspectorInstrumentation::didFailLoadingImpl(InstrumentingAgents& instrumen
         consoleAgent->didFailLoading(identifier, error); // This should come AFTER resource notification, front-end relies on this.
 }
 
-void InspectorInstrumentation::didFinishXHRLoadingImpl(InstrumentingAgents& instrumentingAgents, ThreadableLoaderClient* client, unsigned long identifier, const String& sourceString, const String& url, const String& sendURL, unsigned sendLineNumber, unsigned sendColumnNumber)
+void InspectorInstrumentation::didFinishXHRLoadingImpl(InstrumentingAgents& instrumentingAgents, ThreadableLoaderClient* client, unsigned long identifier, Optional<String> decodedText, const String& url, const String& sendURL, unsigned sendLineNumber, unsigned sendColumnNumber)
 {
     if (WebConsoleAgent* consoleAgent = instrumentingAgents.webConsoleAgent())
         consoleAgent->didFinishXHRLoading(identifier, url, sendURL, sendLineNumber, sendColumnNumber);
-    if (InspectorNetworkAgent* networkAgent = instrumentingAgents.inspectorNetworkAgent())
-        networkAgent->didFinishXHRLoading(client, identifier, sourceString);
+    if (InspectorNetworkAgent* networkAgent = instrumentingAgents.inspectorNetworkAgent()) {
+        if (decodedText)
+            networkAgent->didFinishXHRLoading(client, identifier, *decodedText);
+    }
 }
 
 void InspectorInstrumentation::didReceiveXHRResponseImpl(InstrumentingAgents& instrumentingAgents, unsigned long identifier)
index 9d76605..1a09c89 100644 (file)
@@ -176,7 +176,7 @@ public:
     static void didReceiveData(Frame*, unsigned long identifier, const char* data, int dataLength, int encodedDataLength);
     static void didFinishLoading(Frame*, DocumentLoader*, unsigned long identifier, double finishTime);
     static void didFailLoading(Frame*, DocumentLoader*, unsigned long identifier, const ResourceError&);
-    static void didFinishXHRLoading(ScriptExecutionContext*, ThreadableLoaderClient*, unsigned long identifier, const String& sourceString, const String& url, const String& sendURL, unsigned sendLineNumber, unsigned sendColumnNumber);
+    static void didFinishXHRLoading(ScriptExecutionContext*, ThreadableLoaderClient*, unsigned long identifier, Optional<String> decodedText, const String& url, const String& sendURL, unsigned sendLineNumber, unsigned sendColumnNumber);
     static void didReceiveXHRResponse(ScriptExecutionContext*, unsigned long identifier);
     static void willLoadXHRSynchronously(ScriptExecutionContext*);
     static void didLoadXHRSynchronously(ScriptExecutionContext*);
@@ -348,7 +348,7 @@ private:
     static void didFailLoadingImpl(InstrumentingAgents&, unsigned long identifier, DocumentLoader*, const ResourceError&);
     static void willLoadXHRImpl(InstrumentingAgents&, ThreadableLoaderClient*, const String&, const URL&, bool, RefPtr<FormData>&&, const HTTPHeaderMap&, bool);
     static void didFailXHRLoadingImpl(InstrumentingAgents&, ThreadableLoaderClient*);
-    static void didFinishXHRLoadingImpl(InstrumentingAgents&, ThreadableLoaderClient*, unsigned long identifier, const String& sourceString, const String& url, const String& sendURL, unsigned sendLineNumber, unsigned sendColumnNumber);
+    static void didFinishXHRLoadingImpl(InstrumentingAgents&, ThreadableLoaderClient*, unsigned long identifier, Optional<String> decodedText, const String& url, const String& sendURL, unsigned sendLineNumber, unsigned sendColumnNumber);
     static void didReceiveXHRResponseImpl(InstrumentingAgents&, unsigned long identifier);
     static void willLoadXHRSynchronouslyImpl(InstrumentingAgents&);
     static void didLoadXHRSynchronouslyImpl(InstrumentingAgents&);
@@ -916,10 +916,10 @@ inline void InspectorInstrumentation::didFailLoading(Frame* frame, DocumentLoade
         didFailLoadingImpl(*instrumentingAgents, identifier, loader, error);
 }
 
-inline void InspectorInstrumentation::didFinishXHRLoading(ScriptExecutionContext* context, ThreadableLoaderClient* client, unsigned long identifier, const String& sourceString, const String& url, const String& sendURL, unsigned sendLineNumber, unsigned sendColumnNumber)
+inline void InspectorInstrumentation::didFinishXHRLoading(ScriptExecutionContext* context, ThreadableLoaderClient* client, unsigned long identifier, Optional<String> decodedText, const String& url, const String& sendURL, unsigned sendLineNumber, unsigned sendColumnNumber)
 {
     if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForContext(context))
-        didFinishXHRLoadingImpl(*instrumentingAgents, client, identifier, sourceString, url, sendURL, sendLineNumber, sendColumnNumber);
+        didFinishXHRLoadingImpl(*instrumentingAgents, client, identifier, decodedText, url, sendURL, sendLineNumber, sendColumnNumber);
 }
 
 inline void InspectorInstrumentation::didReceiveXHRResponse(ScriptExecutionContext* context, unsigned long identifier)
index f09f49b..2dbad58 100644 (file)
@@ -448,9 +448,9 @@ void InspectorNetworkAgent::didReceiveScriptResponse(unsigned long identifier)
     m_resourcesData->setResourceType(IdentifiersFactory::requestId(identifier), InspectorPageAgent::ScriptResource);
 }
 
-void InspectorNetworkAgent::didFinishXHRLoading(ThreadableLoaderClient*, unsigned long identifier, const String& sourceString)
+void InspectorNetworkAgent::didFinishXHRLoading(ThreadableLoaderClient*, unsigned long identifier, const String& decodedText)
 {
-    m_resourcesData->setResourceContent(IdentifiersFactory::requestId(identifier), sourceString);
+    m_resourcesData->setResourceContent(IdentifiersFactory::requestId(identifier), decodedText);
 }
 
 void InspectorNetworkAgent::didReceiveXHRResponse(unsigned long identifier)
index 534cef7..2616908 100644 (file)
@@ -82,7 +82,7 @@ public:
     void didFinishLoading(unsigned long identifier, DocumentLoader&, double finishTime);
     void didFailLoading(unsigned long identifier, DocumentLoader&, const ResourceError&);
     void didLoadResourceFromMemoryCache(DocumentLoader&, CachedResource&);
-    void didFinishXHRLoading(ThreadableLoaderClient*, unsigned long identifier, const String& sourceString);
+    void didFinishXHRLoading(ThreadableLoaderClient*, unsigned long identifier, const String& decodedText);
     void didReceiveXHRResponse(unsigned long identifier);
     void willLoadXHRSynchronously();
     void didLoadXHRSynchronously();
index 270404a..c0b5a58 100644 (file)
@@ -1006,7 +1006,10 @@ void XMLHttpRequest::didFinishLoading(unsigned long identifier, double)
 
     m_responseBuilder.shrinkToFit();
 
-    InspectorInstrumentation::didFinishXHRLoading(scriptExecutionContext(), this, identifier, m_responseBuilder.toStringPreserveCapacity(), m_url, m_lastSendURL, m_lastSendLineNumber, m_lastSendColumnNumber);
+    Optional<String> decodedText;
+    if (!m_binaryResponseBuilder)
+        decodedText = m_responseBuilder.toStringPreserveCapacity();
+    InspectorInstrumentation::didFinishXHRLoading(scriptExecutionContext(), this, identifier, decodedText, m_url, m_lastSendURL, m_lastSendLineNumber, m_lastSendColumnNumber);
 
     bool hadLoader = m_loader;
     m_loader = nullptr;