[DevTools] Fix frontend host race and assert.

[DevTools] Fix frontend host race and assert.

When quickly reloading DevTools, on-load messages from frontend
like Console.enable are dispatched from both page loads, but only the
last batch (from the last reload) should be dispatched. Otherwise,
backend sees messages with the same id, and frontend receives multiple
responses, some of them even before load producing
"dispatchMessage is undefined" error.

Also, reloading frontend violates an assert in DevToolsHost.cpp because of
no proper cleanup in WebDevToolsFrontendImpl.

Drive-by: cleaning up WebDevToolsFrontend and DevToolsHost API.

BUG=566929
Committed: https://crrev.com/5c7f1ac926777bcae556dad72be6c45842e564bf
Cr-Commit-Position: refs/heads/master@{#363726}

[dgozman, 2015-12-07 21:29:48]

Description was changed from

==========
[DevTools] Fix frontend host race and assert.

When quickly reloading DevTools itself, stale messages get dispatched twice
from the frontend side and unexpcted results are coming. This produces
"dispatchMessage is undefined" error.

Also, reloading frontend violates an assert in DevToolsHost.cpp because of
no proper cleanup in WebDevToolsFrontendImpl.

Drive-by: cleaning up WebDevToolsFrontend API.

BUG=566929
==========

to

==========
[DevTools] Fix frontend host race and assert.

When quickly reloading DevTools itself, stale messages get dispatched twice
from the frontend side and unexpcted results are coming. This produces
"dispatchMessage is undefined" error.

Also, reloading frontend violates an assert in DevToolsHost.cpp because of
no proper cleanup in WebDevToolsFrontendImpl.

Drive-by: cleaning up WebDevToolsFrontend and DevToolsHost API.

BUG=566929
==========

[dgozman, 2015-12-07 21:29:59]

dgozman@chromium.org changed reviewers:
+ pfeldman@chromium.org

[dgozman, 2015-12-07 21:30:00]

Take a look please.

[pfeldman, 2015-12-07 21:38:42]

https://codereview.chromium.org/1504763004/diff/1/chrome/browser/devtools/dev...
File chrome/browser/devtools/devtools_ui_bindings.cc (right):

https://codereview.chromium.org/1504763004/diff/1/chrome/browser/devtools/dev...
chrome/browser/devtools/devtools_ui_bindings.cc:897: if (agent_host_.get() &&
!frontend_reattaching_)
When does this happen and why should it be ignored?

https://codereview.chromium.org/1504763004/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebDevToolsFrontendImpl.cpp (right):

https://codereview.chromium.org/1504763004/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebDevToolsFrontendImpl.cpp:64: if
(m_devtoolsHost)
Do you have an idea on what is retaining it?

[dgozman, 2015-12-07 21:46:40]

https://codereview.chromium.org/1504763004/diff/1/chrome/browser/devtools/dev...
File chrome/browser/devtools/devtools_ui_bindings.cc (right):

https://codereview.chromium.org/1504763004/diff/1/chrome/browser/devtools/dev...
chrome/browser/devtools/devtools_ui_bindings.cc:897: if (agent_host_.get() &&
!frontend_reattaching_)
On 2015/12/07 21:38:42, pfeldman wrote:
> When does this happen and why should it be ignored?

Happens in Reattach. First paragraph of the patch description answers the second
question.

https://codereview.chromium.org/1504763004/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebDevToolsFrontendImpl.cpp (right):

https://codereview.chromium.org/1504763004/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebDevToolsFrontendImpl.cpp:64: if
(m_devtoolsHost)
On 2015/12/07 21:38:42, pfeldman wrote:
> Do you have an idea on what is retaining it?

I think bindings. Or oilpan destructing it later. It's refcounted, so we have to
account for that.

[pfeldman, 2015-12-07 21:48:36]

https://codereview.chromium.org/1504763004/diff/1/chrome/browser/devtools/dev...
File chrome/browser/devtools/devtools_ui_bindings.cc (right):

https://codereview.chromium.org/1504763004/diff/1/chrome/browser/devtools/dev...
chrome/browser/devtools/devtools_ui_bindings.cc:897: if (agent_host_.get() &&
!frontend_reattaching_)
But how can it be dispatched twice?

[dgozman, 2015-12-07 22:03:45]

Description was changed from

==========
[DevTools] Fix frontend host race and assert.

When quickly reloading DevTools itself, stale messages get dispatched twice
from the frontend side and unexpcted results are coming. This produces
"dispatchMessage is undefined" error.

Also, reloading frontend violates an assert in DevToolsHost.cpp because of
no proper cleanup in WebDevToolsFrontendImpl.

Drive-by: cleaning up WebDevToolsFrontend and DevToolsHost API.

BUG=566929
==========

to

==========
[DevTools] Fix frontend host race and assert.

When quickly reloading DevTools, on-load messages from frontend
like Console.enable are dispatched from both page loads, but only the
last batch (from the last reload) should be dispatched. Otherwise,
backend sees messages with the same id, and frontend receives multiple
responses, some of them even before load producing
"dispatchMessage is undefined" error.

Also, reloading frontend violates an assert in DevToolsHost.cpp because of
no proper cleanup in WebDevToolsFrontendImpl.

Drive-by: cleaning up WebDevToolsFrontend and DevToolsHost API.

BUG=566929
==========

[dgozman, 2015-12-07 22:03:53]

https://codereview.chromium.org/1504763004/diff/1/chrome/browser/devtools/dev...
File chrome/browser/devtools/devtools_ui_bindings.cc (right):

https://codereview.chromium.org/1504763004/diff/1/chrome/browser/devtools/dev...
chrome/browser/devtools/devtools_ui_bindings.cc:897: if (agent_host_.get() &&
!frontend_reattaching_)
On 2015/12/07 21:48:36, pfeldman wrote:
> But how can it be dispatched twice?

Sorry, it wasn't clear. It's not the same message dispatched twice, but one more
similar message. I've updated description.

[dgozman, 2015-12-08 00:35:03]

PTAL

[pfeldman, 2015-12-08 00:36:19]

lgtm

[dgozman, 2015-12-08 01:36:26]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2015-12-08 01:36:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1504763004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1504763004/20001

[commit-bot: I haz the power, 2015-12-08 03:56:54]

Description was changed from

==========
[DevTools] Fix frontend host race and assert.

When quickly reloading DevTools, on-load messages from frontend
like Console.enable are dispatched from both page loads, but only the
last batch (from the last reload) should be dispatched. Otherwise,
backend sees messages with the same id, and frontend receives multiple
responses, some of them even before load producing
"dispatchMessage is undefined" error.

Also, reloading frontend violates an assert in DevToolsHost.cpp because of
no proper cleanup in WebDevToolsFrontendImpl.

Drive-by: cleaning up WebDevToolsFrontend and DevToolsHost API.

BUG=566929
==========

to

==========
[DevTools] Fix frontend host race and assert.

When quickly reloading DevTools, on-load messages from frontend
like Console.enable are dispatched from both page loads, but only the
last batch (from the last reload) should be dispatched. Otherwise,
backend sees messages with the same id, and frontend receives multiple
responses, some of them even before load producing
"dispatchMessage is undefined" error.

Also, reloading frontend violates an assert in DevToolsHost.cpp because of
no proper cleanup in WebDevToolsFrontendImpl.

Drive-by: cleaning up WebDevToolsFrontend and DevToolsHost API.

BUG=566929
==========

[commit-bot: I haz the power, 2015-12-08 03:56:54]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-12-08 03:57:46]

Description was changed from

==========
[DevTools] Fix frontend host race and assert.

When quickly reloading DevTools, on-load messages from frontend
like Console.enable are dispatched from both page loads, but only the
last batch (from the last reload) should be dispatched. Otherwise,
backend sees messages with the same id, and frontend receives multiple
responses, some of them even before load producing
"dispatchMessage is undefined" error.

Also, reloading frontend violates an assert in DevToolsHost.cpp because of
no proper cleanup in WebDevToolsFrontendImpl.

Drive-by: cleaning up WebDevToolsFrontend and DevToolsHost API.

BUG=566929
==========

to

==========
[DevTools] Fix frontend host race and assert.

When quickly reloading DevTools, on-load messages from frontend
like Console.enable are dispatched from both page loads, but only the
last batch (from the last reload) should be dispatched. Otherwise,
backend sees messages with the same id, and frontend receives multiple
responses, some of them even before load producing
"dispatchMessage is undefined" error.

Also, reloading frontend violates an assert in DevToolsHost.cpp because of
no proper cleanup in WebDevToolsFrontendImpl.

Drive-by: cleaning up WebDevToolsFrontend and DevToolsHost API.

BUG=566929

Committed: https://crrev.com/5c7f1ac926777bcae556dad72be6c45842e564bf
Cr-Commit-Position: refs/heads/master@{#363726}
==========

[commit-bot: I haz the power, 2015-12-08 03:57:47]

Patchset 2 (id:??) landed as
https://crrev.com/5c7f1ac926777bcae556dad72be6c45842e564bf
Cr-Commit-Position: refs/heads/master@{#363726}