DevTools: Do not push to frontend messages from worker while it is alive

DevTools: Do not push to frontend messages from worker while it is alive

BUG=

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180165

[vsevik, 2014-08-12 14:25:13]

https://chromiumcodereview.appspot.com/466753002/diff/20001/Source/core/inspe...
File Source/core/inspector/InspectorInstrumentation.idl (right):

https://chromiumcodereview.appspot.com/466753002/diff/20001/Source/core/inspe...
Source/core/inspector/InspectorInstrumentation.idl:400: [Worker, Console]
Let's make an explicit call on the call site instead.

https://chromiumcodereview.appspot.com/466753002/diff/20001/Source/core/worke...
File Source/core/workers/WorkerMessagingProxy.cpp (right):

https://chromiumcodereview.appspot.com/466753002/diff/20001/Source/core/worke...
Source/core/workers/WorkerMessagingProxy.cpp:187: return;
Let's use something like pseudo code below:

ConsoleMessage consoleMessage = ConsoleMessage::create(...)
consoleMessage->setWorkerId(...);
frameConsole->addMessage(consoleMessage);

[sergeyv, 2014-08-12 15:37:54]

https://codereview.chromium.org/466753002/diff/20001/Source/core/inspector/In...
File Source/core/inspector/InspectorInstrumentation.idl (right):

https://codereview.chromium.org/466753002/diff/20001/Source/core/inspector/In...
Source/core/inspector/InspectorInstrumentation.idl:400: [Worker, Console]
On 2014/08/12 14:25:13, vsevik wrote:
> Let's make an explicit call on the call site instead.

Done.

https://codereview.chromium.org/466753002/diff/20001/Source/core/workers/Work...
File Source/core/workers/WorkerMessagingProxy.cpp (right):

https://codereview.chromium.org/466753002/diff/20001/Source/core/workers/Work...
Source/core/workers/WorkerMessagingProxy.cpp:187: return;
On 2014/08/12 14:25:13, vsevik wrote:
> Let's use something like pseudo code below:
> 
> ConsoleMessage consoleMessage = ConsoleMessage::create(...)
> consoleMessage->setWorkerId(...);
> frameConsole->addMessage(consoleMessage);

Done.

[vsevik, 2014-08-12 15:46:08]

lgtm with a few nits.

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/frame...
File Source/core/frame/Console.h (right):

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/frame...
Source/core/frame/Console.h:56: virtual void reportMessageToClient(MessageLevel,
const String& message, PassRefPtrWillBeRawPtr<ScriptCallStack>) OVERRIDE;
revert this

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/frame...
File Source/core/frame/ConsoleBase.h (right):

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/frame...
Source/core/frame/ConsoleBase.h:74: virtual void
reportMessageToClient(MessageLevel, const String& message,
PassRefPtrWillBeRawPtr<ScriptCallStack>) = 0;
revert this

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/inspe...
File Source/core/inspector/ConsoleMessage.h (right):

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/inspe...
Source/core/inspector/ConsoleMessage.h:48: WorkerGlobalScopeProxy* getWorkerId()
{ return m_workerProxy; }
workerId()

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/inspe...
File Source/core/inspector/InspectorConsoleMessage.h (right):

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/inspe...
Source/core/inspector/InspectorConsoleMessage.h:63: WorkerGlobalScopeProxy*
getWorkerGlobalScopeProxy() { return m_workerProxy; }
no get* please

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/inspe...
File Source/core/inspector/InspectorInstrumentation.idl (right):

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/inspe...
Source/core/inspector/InspectorInstrumentation.idl:514: //FIXME: remove when we
move everything from InspectorConsoleAgent to FrameConsole
s/everything/console message storage/

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/worke...
File Source/core/workers/WorkerMessagingProxy.cpp (right):

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/worke...
Source/core/workers/WorkerMessagingProxy.cpp:276: // FIXME: This need to be
revisited when we support nested worker one day
Let's extract a method here.

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/worke...
File Source/core/workers/WorkerMessagingProxy.h (right):

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/worke...
Source/core/workers/WorkerMessagingProxy.h:47: class ConsoleMessage;
revert this

[vsevik, 2014-08-12 15:49:31]

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/worke...
File Source/core/workers/WorkerMessagingProxy.cpp (right):

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/worke...
Source/core/workers/WorkerMessagingProxy.cpp:281:
frame->console().workerGlobalScopeTerminated(this);
adoptWorkerConsoleMsssages

[vsevik, 2014-08-12 15:49:54]

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/inspe...
File Source/core/inspector/InspectorInstrumentation.idl (right):

https://chromiumcodereview.appspot.com/466753002/diff/40001/Source/core/inspe...
Source/core/inspector/InspectorInstrumentation.idl:516: void
notifyWorkerGlobalScopeTerminated(ExecutionContext* context,
WorkerGlobalScopeProxy* proxy);
adoptWorkerConsoleMessages

[sergeyv, 2014-08-12 16:00:59]

https://codereview.chromium.org/466753002/diff/40001/Source/core/frame/Console.h
File Source/core/frame/Console.h (right):

https://codereview.chromium.org/466753002/diff/40001/Source/core/frame/Consol...
Source/core/frame/Console.h:56: virtual void reportMessageToClient(MessageLevel,
const String& message, PassRefPtrWillBeRawPtr<ScriptCallStack>) OVERRIDE;
On 2014/08/12 15:46:07, vsevik wrote:
> revert this

Done.

https://codereview.chromium.org/466753002/diff/40001/Source/core/frame/Consol...
File Source/core/frame/ConsoleBase.h (right):

https://codereview.chromium.org/466753002/diff/40001/Source/core/frame/Consol...
Source/core/frame/ConsoleBase.h:74: virtual void
reportMessageToClient(MessageLevel, const String& message,
PassRefPtrWillBeRawPtr<ScriptCallStack>) = 0;
On 2014/08/12 15:46:07, vsevik wrote:
> revert this

Done.

https://codereview.chromium.org/466753002/diff/40001/Source/core/inspector/Co...
File Source/core/inspector/ConsoleMessage.h (right):

https://codereview.chromium.org/466753002/diff/40001/Source/core/inspector/Co...
Source/core/inspector/ConsoleMessage.h:48: WorkerGlobalScopeProxy* getWorkerId()
{ return m_workerProxy; }
On 2014/08/12 15:46:07, vsevik wrote:
> workerId()

Done.

https://codereview.chromium.org/466753002/diff/40001/Source/core/inspector/In...
File Source/core/inspector/InspectorConsoleMessage.h (right):

https://codereview.chromium.org/466753002/diff/40001/Source/core/inspector/In...
Source/core/inspector/InspectorConsoleMessage.h:63: WorkerGlobalScopeProxy*
getWorkerGlobalScopeProxy() { return m_workerProxy; }
On 2014/08/12 15:46:07, vsevik wrote:
> no get* please

Done.

https://codereview.chromium.org/466753002/diff/40001/Source/core/inspector/In...
File Source/core/inspector/InspectorInstrumentation.idl (right):

https://codereview.chromium.org/466753002/diff/40001/Source/core/inspector/In...
Source/core/inspector/InspectorInstrumentation.idl:514: //FIXME: remove when we
move everything from InspectorConsoleAgent to FrameConsole
On 2014/08/12 15:46:07, vsevik wrote:
> s/everything/console message storage/

Done.

https://codereview.chromium.org/466753002/diff/40001/Source/core/inspector/In...
Source/core/inspector/InspectorInstrumentation.idl:516: void
notifyWorkerGlobalScopeTerminated(ExecutionContext* context,
WorkerGlobalScopeProxy* proxy);
On 2014/08/12 15:49:54, vsevik wrote:
> adoptWorkerConsoleMessages

Done.

https://codereview.chromium.org/466753002/diff/40001/Source/core/workers/Work...
File Source/core/workers/WorkerMessagingProxy.cpp (right):

https://codereview.chromium.org/466753002/diff/40001/Source/core/workers/Work...
Source/core/workers/WorkerMessagingProxy.cpp:276: // FIXME: This need to be
revisited when we support nested worker one day
On 2014/08/12 15:46:07, vsevik wrote:
> Let's extract a method here.

Done.

https://codereview.chromium.org/466753002/diff/40001/Source/core/workers/Work...
Source/core/workers/WorkerMessagingProxy.cpp:281:
frame->console().workerGlobalScopeTerminated(this);
On 2014/08/12 15:49:31, vsevik wrote:
> adoptWorkerConsoleMsssages

Done.

https://codereview.chromium.org/466753002/diff/40001/Source/core/workers/Work...
File Source/core/workers/WorkerMessagingProxy.h (right):

https://codereview.chromium.org/466753002/diff/40001/Source/core/workers/Work...
Source/core/workers/WorkerMessagingProxy.h:47: class ConsoleMessage;
On 2014/08/12 15:46:07, vsevik wrote:
> revert this

Done.

[sergeyv, 2014-08-12 16:03:33]

The CQ bit was checked by sergeyv@chromium.org

[commit-bot: I haz the power, 2014-08-12 16:04:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sergeyv@chromium.org/466753002/60001

[commit-bot: I haz the power, 2014-08-12 18:02:21]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/2...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/19492)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22319)

[commit-bot: I haz the power, 2014-08-12 18:04:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-12 18:04:36]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/19505)

[sergeyv, 2014-08-12 22:05:37]

The CQ bit was checked by sergeyv@chromium.org

[commit-bot: I haz the power, 2014-08-12 22:07:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sergeyv@chromium.org/466753002/60001

[commit-bot: I haz the power, 2014-08-12 22:08:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-12 22:08:41]

Failed to apply patch for Source/core/inspector/InspectorConsoleAgent.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/inspector/InspectorConsoleAgent.cpp
  Hunk #2 FAILED at 170.
  1 out of 2 hunks FAILED -- saving rejects to file
Source/core/inspector/InspectorConsoleAgent.cpp.rej

Patch:       Source/core/inspector/InspectorConsoleAgent.cpp
Index: Source/core/inspector/InspectorConsoleAgent.cpp
diff --git a/Source/core/inspector/InspectorConsoleAgent.cpp
b/Source/core/inspector/InspectorConsoleAgent.cpp
index
b89257e61af1115655af7633c757e1d420d8681d..44ec14a2541c892e9d535c36919e1e794b4db4e8
100644
--- a/Source/core/inspector/InspectorConsoleAgent.cpp
+++ b/Source/core/inspector/InspectorConsoleAgent.cpp
@@ -44,6 +44,7 @@
 #include "core/inspector/ScriptCallStack.h"
 #include "core/loader/DocumentLoader.h"
 #include "core/page/Page.h"
+#include "core/workers/WorkerGlobalScopeProxy.h"
 #include "platform/network/ResourceError.h"
 #include "platform/network/ResourceResponse.h"
 #include "wtf/CurrentTime.h"
@@ -169,11 +170,22 @@ void InspectorConsoleAgent::clearFrontend()
 
 void InspectorConsoleAgent::addMessageToConsole(ConsoleMessage* consoleMessage)
 {
+    InspectorConsoleMessage* message;
     if (consoleMessage->callStack()) {
-        addConsoleMessage(adoptPtr(new
InspectorConsoleMessage(consoleMessage->source(), LogMessageType,
consoleMessage->level(), consoleMessage->message(), consoleMessage->callStack(),
consoleMessage->requestIdentifier())));
+        message = new InspectorConsoleMessage(consoleMessage->source(),
LogMessageType, consoleMessage->level(), consoleMessage->message(),
consoleMessage->callStack(), consoleMessage->requestIdentifier());
     } else {
         bool canGenerateCallStack = !isWorkerAgent() && m_frontend;
-        addConsoleMessage(adoptPtr(new
InspectorConsoleMessage(canGenerateCallStack, consoleMessage->source(),
LogMessageType, consoleMessage->level(), consoleMessage->message(),
consoleMessage->url(), consoleMessage->lineNumber(),
consoleMessage->columnNumber(), consoleMessage->scriptState(),
consoleMessage->requestIdentifier())));
+        message = new InspectorConsoleMessage(canGenerateCallStack,
consoleMessage->source(), LogMessageType, consoleMessage->level(),
consoleMessage->message(), consoleMessage->url(), consoleMessage->lineNumber(),
consoleMessage->columnNumber(), consoleMessage->scriptState(),
consoleMessage->requestIdentifier());
+    }
+    message->setWorkerGlobalScopeProxy(consoleMessage->workerId());
+    addConsoleMessage(adoptPtr(message));
+}
+
+void InspectorConsoleAgent::adoptWorkerConsoleMessages(WorkerGlobalScopeProxy*
proxy)
+{
+    for (size_t i = 0; i < m_consoleMessages.size(); i++) {
+        if (m_consoleMessages[i]->workerGlobalScopeProxy() == proxy)
+            m_consoleMessages[i]->setWorkerGlobalScopeProxy(nullptr);
     }
 }

[sergeyv, 2014-08-13 07:30:18]

The CQ bit was checked by sergeyv@chromium.org

[commit-bot: I haz the power, 2014-08-13 07:31:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sergeyv@chromium.org/466753002/80001

[commit-bot: I haz the power, 2014-08-13 08:38:15]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/2...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/19593)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22478)

[sergeyv, 2014-08-13 09:25:29]

The CQ bit was checked by sergeyv@chromium.org

[commit-bot: I haz the power, 2014-08-13 09:26:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sergeyv@chromium.org/466753002/120001

[commit-bot: I haz the power, 2014-08-13 10:27:55]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22498)

[commit-bot: I haz the power, 2014-08-13 11:11:32]

Change committed as 180165