Make the WebSocket fail messages meaningful

Source/modules/websockets/MainThreadWebSocketChannel.cpp

Index: Source/modules/websockets/MainThreadWebSocketChannel.cpp
diff --git a/Source/modules/websockets/MainThreadWebSocketChannel.cpp b/Source/modules/websockets/MainThreadWebSocketChannel.cpp
index ba388d054fddf47b3c45b957f5c2a9c4ccf61e3f..6aba196a84070b93b8a63641b29171c441d35466 100644
--- a/Source/modules/websockets/MainThreadWebSocketChannel.cpp
+++ b/Source/modules/websockets/MainThreadWebSocketChannel.cpp
@@ -45,6 +45,7 @@
 #include "Logging.h"
 #include "Page.h"
 #include "ProgressTracker.h"
+#include "ScriptCallStackFactory.h"
 #include "ScriptCallStack.h"

[tyoshino (SeeGerritForStatus), 2013/04/19 14:18:10]

swap order of these two

[yhirano, 2013/04/22 05:18:29]

Done.

 #include "ScriptExecutionContext.h"
 #include "Settings.h"
@@ -86,6 +87,7 @@ MainThreadWebSocketChannel::MainThreadWebSocketChannel(Document* document, WebSo
     , m_closeEventCode(CloseEventCodeAbnormalClosure)
     , m_outgoingFrameQueueStatus(OutgoingFrameQueueOpen)
     , m_blobLoaderStatus(BlobLoaderNotStarted)
+    , m_callframeAtConnection("", "", 0)
 {
     if (Page* page = m_document->page())
         m_identifier = page->progress()->createUniqueIdentifier();
@@ -108,6 +110,8 @@ void MainThreadWebSocketChannel::connect(const KURL& url, const String& protocol
         InspectorInstrumentation::didCreateWebSocket(m_document, m_identifier, url, m_document->url(), protocol);
     ref();
     m_handle = SocketStreamHandle::create(m_handshake->url(), this);
+    RefPtr<ScriptCallStack> callstack = createScriptCallStackForConsole(1);
+    m_callframeAtConnection = callstack && callstack->size() > 0 ? callstack->at(0) : ScriptCallFrame("", m_handshake->clientOrigin(), 0);
 }
 
 String MainThreadWebSocketChannel::subprotocol()
@@ -196,7 +200,19 @@ void MainThreadWebSocketChannel::fail(const String& reason)
     ASSERT(!m_suspended);
     if (m_document) {
         InspectorInstrumentation::didReceiveWebSocketFrameError(m_document, m_identifier, reason);
-        m_document->addConsoleMessage(NetworkMessageSource, ErrorMessageLevel, "WebSocket connection to '" + m_handshake->url().elidedString() + "' failed: " + reason);
+        const String message = "WebSocket connection to '" + m_handshake->url().elidedString() + "' failed: " + reason;
+        RefPtr<ScriptCallStack> callstack = createScriptCallStackForConsole(1);
+        if (callstack && callstack->size() > 0) {
+            // We are in a JS callstack.
+            // So, the addConsoleMessage method will show the stack apropriately.

[tyoshino (SeeGerritForStatus), 2013/04/19 14:18:10]

apro -> appro

[yhirano, 2013/04/22 05:18:29]

Done.

+            m_document->addConsoleMessage(JSMessageSource, ErrorMessageLevel, message);

[tyoshino (SeeGerritForStatus), 2013/04/19 14:18:10]

Could you please check which of JSMessageSource and NetworkMessageSource is
suitable for this call. AFAIK, this is converted into json and passed to the
inspector frontend javascript, but looks like not used yet.

[yhirano, 2013/04/22 05:18:29]

devtools JS (Source/devtools/front_end/ConsoleMessage.js) would change the
appearance of logs according to the message source parameter.
WebSocket.cpp uses JSMessageSource, too.
According to codesearch, no one but MainThreadWebSocketChannel uses
NetworkMessageSource.

[tyoshino (SeeGerritForStatus), 2013/04/22 06:32:34]

XHR is using it too but in InspectorConsoleAgent.

This should be because the only objects that involve network event are XHR,
resource loading and WebSocket.

I think we should use NetworkMessageSource when fail() is invoked from
processBuffer(), processFrame(), didReceiveSocketStreamData() and maybe
didOpenSocketStream(). You can do this in a separate CL.

For call from didFail, I'm not sure which is the best. Let's use JSMessageSource
for now.

[yhirano, 2013/04/22 07:33:38]

I see, thank you. I will file a bug.

+        } else {
+            // We are not in a JS callstack.
+            // Then show the source file and the line number at the connection initiation.
+            const String& url = m_callframeAtConnection.sourceURL();
+            unsigned lineNumber = m_callframeAtConnection.lineNumber();
+            static_cast<ScriptExecutionContext*>(m_document)->addConsoleMessage(JSMessageSource, ErrorMessageLevel, message, url, lineNumber, 0, 0);

[tyoshino (SeeGerritForStatus), 2013/04/19 14:18:10]

Without this cast, we cannot call the non-virtual addConsoleMessage?

[yhirano, 2013/04/22 05:18:29]

In C++, a member function of a derived class hides same-name  member functions
of its base class.
Maybe the Document class should declare the non-virtual addMessageConsole
function with the "using" keyword.

[tyoshino (SeeGerritForStatus), 2013/04/22 06:32:34]

OK

+        }
     }
 
     // Hybi-10 specification explicitly states we must not continue to handle incoming data