Title: [234702] trunk
Revision
234702
Author
[email protected]
Date
2018-08-08 10:59:56 -0700 (Wed, 08 Aug 2018)

Log Message

Web Inspector: XHR content sometimes shows as error even though load succeeded
https://bugs.webkit.org/show_bug.cgi?id=188385
<rdar://problem/42646160>

Source/WebCore:

Reviewed by Devin Rousso.

* inspector/agents/InspectorNetworkAgent.cpp:
(WebCore::InspectorNetworkAgent::didReceiveData):
Avoid a double hash lookup in the common case.
Allow synchronous XHR to have text data appended in the normal case.
Allow synchronous XHR to set base64 encoded data right here for non-text data.

* inspector/NetworkResourcesData.h:
(WebCore::NetworkResourcesData::ResourceData::hasBufferedData const):
Getter to see if data is buffered or not for this resource.

* inspector/NetworkResourcesData.cpp:
(WebCore::NetworkResourcesData::maybeAddResourceData):
Return the updated ResourceData to avoid clients having to do a lookup.

LayoutTests:

Reviewed by Devin Rousso.

* http/tests/inspector/network/xhr-response-body-expected.txt:
* http/tests/inspector/network/xhr-response-body.html:
Extend this test to include synchronous XHR for text and non-text resources.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (234701 => 234702)


--- trunk/LayoutTests/ChangeLog	2018-08-08 17:59:48 UTC (rev 234701)
+++ trunk/LayoutTests/ChangeLog	2018-08-08 17:59:56 UTC (rev 234702)
@@ -1,3 +1,15 @@
+2018-08-08  Joseph Pecoraro  <[email protected]>
+
+        Web Inspector: XHR content sometimes shows as error even though load succeeded
+        https://bugs.webkit.org/show_bug.cgi?id=188385
+        <rdar://problem/42646160>
+
+        Reviewed by Devin Rousso.
+
+        * http/tests/inspector/network/xhr-response-body-expected.txt:
+        * http/tests/inspector/network/xhr-response-body.html:
+        Extend this test to include synchronous XHR for text and non-text resources.
+
 2018-08-08  Truitt Savell  <[email protected]>
 
         Adjusting test expectations for imported/blink/fast/text/international-iteration-simple-text.html

Modified: trunk/LayoutTests/http/tests/inspector/network/xhr-response-body-expected.txt (234701 => 234702)


--- trunk/LayoutTests/http/tests/inspector/network/xhr-response-body-expected.txt	2018-08-08 17:59:48 UTC (rev 234701)
+++ trunk/LayoutTests/http/tests/inspector/network/xhr-response-body-expected.txt	2018-08-08 17:59:56 UTC (rev 234702)
@@ -44,3 +44,15 @@
 PASS: Content should be base64Encoded.
 PASS: Image content should be a Blob.
 
+-- Running test case: Network.getResponseBody.XHR.Sync.Text
+PASS: Resource should be XHR type.
+PASS: MIMEType should be 'text/plain'.
+PASS: Content should not be base64Encoded.
+PASS: Text content should match data.txt.
+
+-- Running test case: Network.getResponseBody.XHR.Sync.PNG
+PASS: Resource should be XHR type.
+PASS: MIMEType should be 'image/png'.
+PASS: Content should be base64Encoded.
+PASS: Image content should be a Blob.
+

Modified: trunk/LayoutTests/http/tests/inspector/network/xhr-response-body.html (234701 => 234702)


--- trunk/LayoutTests/http/tests/inspector/network/xhr-response-body.html	2018-08-08 17:59:48 UTC (rev 234701)
+++ trunk/LayoutTests/http/tests/inspector/network/xhr-response-body.html	2018-08-08 17:59:56 UTC (rev 234702)
@@ -4,9 +4,9 @@
 <meta charset="utf-8">
 <script src=""
 <script>
-function xhrGet(url) {
+function xhrGet(url, asyncFlag = true) {
     let xhr = new XMLHttpRequest;
-    xhr.open("GET", url, true);
+    xhr.open("GET", url, asyncFlag);
     xhr.send();
 }
 
@@ -38,6 +38,14 @@
     xhrGet("/resources/square100.png");
 }
 
+function createSyncXHRForText() {
+    xhrGet("resources/data.txt?sync", false);
+}
+
+function createSyncXHRForPNG() {
+    xhrGet("/resources/square100.png?sync", false);
+}
+
 function test()
 {
     let suite = InspectorTest.createAsyncSuite("Network.getResponseBody.XHR");
@@ -74,7 +82,7 @@
     addTestCase({
         name: "Network.getResponseBody.XHR.Text",
         description: "Get text/plain content as text.",
-        _expression_: "createXHRForText()",
+        _expression_: `createXHRForText()`,
         contentHandler({error, sourceCode, content, rawBase64Encoded}) {
             InspectorTest.expectEqual(sourceCode.mimeType, "text/plain", "MIMEType should be 'text/plain'.");
             InspectorTest.expectEqual(rawBase64Encoded, false, "Content should not be base64Encoded.");
@@ -85,7 +93,7 @@
     addTestCase({
         name: "Network.getResponseBody.XHR.HTML",
         description: "Get text/html content as text.",
-        _expression_: "createXHRForHTML()",
+        _expression_: `createXHRForHTML()`,
         contentHandler({error, sourceCode, content, rawBase64Encoded}) {
             InspectorTest.expectEqual(sourceCode.mimeType, "text/html", "MIMEType should be 'text/html'.");
             InspectorTest.expectEqual(rawBase64Encoded, false, "Content should not be base64Encoded.");
@@ -96,7 +104,7 @@
     addTestCase({
         name: "Network.getResponseBody.XHR.JSON",
         description: "Get application/octet-stream content as base64Encoded text.",
-        _expression_: "createXHRForJSON()",
+        _expression_: `createXHRForJSON()`,
         contentHandler({error, sourceCode, content, rawBase64Encoded}) {
             InspectorTest.expectEqual(sourceCode.mimeType, "application/octet-stream", "MIMEType should be 'application/octet-stream'.");
             InspectorTest.expectEqual(rawBase64Encoded, true, "Content should be base64Encoded.");
@@ -109,7 +117,7 @@
     addTestCase({
         name: "Network.getResponseBody.XHR.JSON2",
         description: "Get application/json content as text.",
-        _expression_: "createXHRForJSON2()",
+        _expression_: `createXHRForJSON2()`,
         contentHandler({error, sourceCode, content, rawBase64Encoded}) {
             InspectorTest.expectEqual(sourceCode.mimeType, "application/json", "MIMEType should be 'application/json'.");
             InspectorTest.expectEqual(rawBase64Encoded, false, "Content should not be base64Encoded.");
@@ -120,7 +128,7 @@
     addTestCase({
         name: "Network.getResponseBody.XHR.JSON3",
         description: "Get arbitrary +json content as text.",
-        _expression_: "createXHRForJSON3()",
+        _expression_: `createXHRForJSON3()`,
         contentHandler({error, sourceCode, content, rawBase64Encoded}) {
             InspectorTest.expectEqual(sourceCode.mimeType, "application/vnd.api+json", "MIMEType should be 'application/vnd.api+json'.");
             InspectorTest.expectEqual(rawBase64Encoded, false, "Content should not be base64Encoded.");
@@ -131,7 +139,7 @@
     addTestCase({
         name: "Network.getResponseBody.XHR.SVG",
         description: "Get image/svg+xml content as text.",
-        _expression_: "createXHRForSVG()",
+        _expression_: `createXHRForSVG()`,
         contentHandler({error, sourceCode, content, rawBase64Encoded}) {
             InspectorTest.expectEqual(sourceCode.mimeType, "image/svg+xml", "MIMEType should be 'image/svg+xml'.");
             InspectorTest.expectEqual(rawBase64Encoded, false, "Content should not be base64Encoded.");
@@ -147,7 +155,7 @@
     addTestCase({
         name: "Network.getResponseBody.XHR.PNG",
         description: "Get image/png content.",
-        _expression_: "createXHRForPNG()",
+        _expression_: `createXHRForPNG()`,
         contentHandler({error, sourceCode, content, rawBase64Encoded}) {
             InspectorTest.expectEqual(sourceCode.mimeType, "image/png", "MIMEType should be 'image/png'.");
             InspectorTest.expectEqual(rawBase64Encoded, true, "Content should be base64Encoded.");
@@ -155,6 +163,28 @@
         }
     });
 
+    addTestCase({
+        name: "Network.getResponseBody.XHR.Sync.Text",
+        description: "Get text/plain content as text from a synchronous XHR.",
+        _expression_: `createSyncXHRForText()`,
+        contentHandler({error, sourceCode, content, rawBase64Encoded}) {
+            InspectorTest.expectEqual(sourceCode.mimeType, "text/plain", "MIMEType should be 'text/plain'.");
+            InspectorTest.expectEqual(rawBase64Encoded, false, "Content should not be base64Encoded.");
+            InspectorTest.expectEqual(content, "Plain text resource.\n", "Text content should match data.txt.");
+        }
+    });
+
+    addTestCase({
+        name: "Network.getResponseBody.XHR.Sync.PNG",
+        description: "Get image/png content from a synchronous XHR.",
+        _expression_: `createSyncXHRForPNG()`,
+        contentHandler({error, sourceCode, content, rawBase64Encoded}) {
+            InspectorTest.expectEqual(sourceCode.mimeType, "image/png", "MIMEType should be 'image/png'.");
+            InspectorTest.expectEqual(rawBase64Encoded, true, "Content should be base64Encoded.");
+            InspectorTest.expectThat(content instanceof Blob, "Image content should be a Blob.");
+        }
+    });
+
     suite.runTestCasesAndFinish();
 }
 </script>

Modified: trunk/Source/WebCore/ChangeLog (234701 => 234702)


--- trunk/Source/WebCore/ChangeLog	2018-08-08 17:59:48 UTC (rev 234701)
+++ trunk/Source/WebCore/ChangeLog	2018-08-08 17:59:56 UTC (rev 234702)
@@ -1,3 +1,25 @@
+2018-08-08  Joseph Pecoraro  <[email protected]>
+
+        Web Inspector: XHR content sometimes shows as error even though load succeeded
+        https://bugs.webkit.org/show_bug.cgi?id=188385
+        <rdar://problem/42646160>
+
+        Reviewed by Devin Rousso.
+
+        * inspector/agents/InspectorNetworkAgent.cpp:
+        (WebCore::InspectorNetworkAgent::didReceiveData):
+        Avoid a double hash lookup in the common case.
+        Allow synchronous XHR to have text data appended in the normal case.
+        Allow synchronous XHR to set base64 encoded data right here for non-text data.
+
+        * inspector/NetworkResourcesData.h:
+        (WebCore::NetworkResourcesData::ResourceData::hasBufferedData const):
+        Getter to see if data is buffered or not for this resource.
+
+        * inspector/NetworkResourcesData.cpp:
+        (WebCore::NetworkResourcesData::maybeAddResourceData):
+        Return the updated ResourceData to avoid clients having to do a lookup.
+
 2018-08-08  Sihui Liu  <[email protected]>
 
         Assertion failed in Webcore::Process::setIdentifier()

Modified: trunk/Source/WebCore/inspector/NetworkResourcesData.cpp (234701 => 234702)


--- trunk/Source/WebCore/inspector/NetworkResourcesData.cpp	2018-08-08 17:59:48 UTC (rev 234701)
+++ trunk/Source/WebCore/inspector/NetworkResourcesData.cpp	2018-08-08 17:59:56 UTC (rev 234702)
@@ -219,19 +219,19 @@
     return false;
 }
 
-void NetworkResourcesData::maybeAddResourceData(const String& requestId, const char* data, size_t dataLength)
+NetworkResourcesData::ResourceData const* NetworkResourcesData::maybeAddResourceData(const String& requestId, const char* data, size_t dataLength)
 {
     ResourceData* resourceData = resourceDataForRequestId(requestId);
     if (!resourceData)
-        return;
+        return nullptr;
 
     if (!shouldBufferResourceData(*resourceData))
-        return;
+        return resourceData;
 
     if (resourceData->dataLength() + dataLength > m_maximumSingleResourceContentSize)
         m_contentSize -= resourceData->evictContent();
     if (resourceData->isContentEvicted())
-        return;
+        return resourceData;
 
     if (ensureFreeSpace(dataLength) && !resourceData->isContentEvicted()) {
         m_requestIdsDeque.append(requestId);
@@ -238,6 +238,8 @@
         resourceData->appendData(data, dataLength);
         m_contentSize += dataLength;
     }
+
+    return resourceData;
 }
 
 void NetworkResourcesData::maybeDecodeDataToContent(const String& requestId)

Modified: trunk/Source/WebCore/inspector/NetworkResourcesData.h (234701 => 234702)


--- trunk/Source/WebCore/inspector/NetworkResourcesData.h	2018-08-08 17:59:48 UTC (rev 234701)
+++ trunk/Source/WebCore/inspector/NetworkResourcesData.h	2018-08-08 17:59:56 UTC (rev 234702)
@@ -90,6 +90,8 @@
         bool forceBufferData() const { return m_forceBufferData; }
         void setForceBufferData(bool force) { m_forceBufferData = force; }
 
+        bool hasBufferedData() const { return m_dataBuffer; }
+
     private:
         bool hasData() const { return m_dataBuffer; }
         size_t dataLength() const;
@@ -122,7 +124,7 @@
     void setResourceType(const String& requestId, InspectorPageAgent::ResourceType);
     InspectorPageAgent::ResourceType resourceType(const String& requestId);
     void setResourceContent(const String& requestId, const String& content, bool base64Encoded = false);
-    void maybeAddResourceData(const String& requestId, const char* data, size_t dataLength);
+    ResourceData const* maybeAddResourceData(const String& requestId, const char* data, size_t dataLength);
     void maybeDecodeDataToContent(const String& requestId);
     void addCachedResource(const String& requestId, CachedResource*);
     void addResourceSharedBuffer(const String& requestId, RefPtr<SharedBuffer>&&, const String& textEncodingName);

Modified: trunk/Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp (234701 => 234702)


--- trunk/Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp	2018-08-08 17:59:48 UTC (rev 234701)
+++ trunk/Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp	2018-08-08 17:59:56 UTC (rev 234702)
@@ -486,9 +486,13 @@
     String requestId = IdentifiersFactory::requestId(identifier);
 
     if (data) {
-        NetworkResourcesData::ResourceData const* resourceData = m_resourcesData->data(requestId);
-        if (resourceData && !m_loadingXHRSynchronously)
-            m_resourcesData->maybeAddResourceData(requestId, data, dataLength);
+        NetworkResourcesData::ResourceData const* resourceData = m_resourcesData->maybeAddResourceData(requestId, data, dataLength);
+
+        // For a synchronous XHR, if we didn't add data then we can apply it here as base64 encoded content.
+        // Often the data is text and we would have a decoder, but for non-text we won't have a decoder.
+        // Sync XHRs may not have a cached resource, while non-sync XHRs usually transfer data over on completion.
+        if (m_loadingXHRSynchronously && resourceData && !resourceData->hasBufferedData() && !resourceData->cachedResource())
+            m_resourcesData->setResourceContent(requestId, base64Encode(data, dataLength), true);
     }
 
     m_frontendDispatcher->dataReceived(requestId, timestamp(), dataLength, encodedDataLength);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to