Bug 73127 - Web Inspector: remove disconnectFromBackend from the protocol.
: Web Inspector: remove disconnectFromBackend from the protocol.
Status: RESOLVED FIXED
: WebKit
Web Inspector (Deprecated)
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
: 73158 73188
:
  Show dependency treegraph
 
Reported: 2011-11-25 05:18 PST by
Modified: 2011-11-28 01:31 PST (History)


Attachments
Patch (31.45 KB, patch)
2011-11-25 05:21 PST, Pavel Feldman
no flags Review Patch | Details | Formatted Diff | Diff
[Patch] with mac bots fixed (31.94 KB, patch)
2011-11-25 06:31 PST, Pavel Feldman
no flags Review Patch | Details | Formatted Diff | Diff
[Patch] rebaselined (31.87 KB, patch)
2011-11-25 07:57 PST, Pavel Feldman
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-11-25 05:18:06 PST
We should instead use InspectorClient::closeInspectorFrontend that would be closing the front-end window.
------- Comment #1 From 2011-11-25 05:21:49 PST -------
Created an attachment (id=116602) [details]
Patch
------- Comment #2 From 2011-11-25 06:31:33 PST -------
Created an attachment (id=116614) [details]
[Patch] with mac bots fixed
------- Comment #3 From 2011-11-25 07:07:43 PST -------
(From update of attachment 116614 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=116614&action=review

> Source/WebCore/inspector/InspectorController.cpp:275
> +    m_inspectorClient->closeInspectorFrontend();

Instead of introducing a new method on client I'd suggest we move InspectorController::close implementation to WebKit and call corresponding  closeInspectorFrontend implementation there without going through WebCore. What prevents you from doing so?

> Source/WebKit/chromium/src/WebDevToolsAgentImpl.cpp:267
> +void WebDevToolsAgentImpl::closeInspectorFrontend()

Let's remove WebDevToolsAgentImpl inheritance from InspectorClient as we have a separate implementation of the client in InspectorClientImpl, then we'll be able to remove all these empty methods.

> Source/WebKit/chromium/src/WebDevToolsAgentImpl.cpp:-384
> -    inspector.disconnectFromBackend();

The API method should also be renamed to disconnectFromWorkerEventAsText()
------- Comment #4 From 2011-11-25 07:50:59 PST -------
InspectorController::close implementation to WebKit and call corresponding  closeInspectorFrontend implementation there without going through WebCore. What prevents you from doing so?

I share your concern and I looked into doing it, but quickly found out that I'll need to introduce a lot of plumbing to make it happen on all ports. Most of the DRTs and WK1 clients have access to a page and do page->inspectorController()->close(). Making them look up corresponding WebKit inspector instance would result in moderate amount of platform-specific work. Given that InspectorClient interface is in place, I'd rather keep ports' implementation aligned.
------- Comment #5 From 2011-11-25 07:57:36 PST -------
Created an attachment (id=116620) [details]
[Patch] rebaselined
------- Comment #6 From 2011-11-26 07:31:08 PST -------
(From update of attachment 116620 [details])
Clearing flags on attachment: 116620

Committed r101193: <http://trac.webkit.org/changeset/101193>
------- Comment #7 From 2011-11-26 07:31:17 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #8 From 2011-11-27 07:55:03 PST -------
Committed r101201: <http://trac.webkit.org/changeset/101201>
------- Comment #9 From 2011-11-28 01:31:05 PST -------
This has caused crashes after closing any tab with WebKit2: bug 73183.