Bug 73127

Summary:

Web Inspector: remove disconnectFromBackend from the protocol.

Product:

WebKit

Reporter:

Pavel Feldman <pfeldman>

Component:

Web Inspector (Deprecated)

Assignee:

Pavel Feldman <pfeldman>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

apavlov, bweinstein, japhet, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

73158, 73188

Bug Blocks:

Attachments:

Description	Flags
Patch	none
[Patch] with mac bots fixed	none
[Patch] rebaselined	none

Pavel Feldman

Reported 2011-11-25 05:18:06 PST

We should instead use InspectorClient::closeInspectorFrontend that would be closing the front-end window.

Attachments
Patch (31.45 KB, patch) 2011-11-25 05:21 PST, Pavel Feldman	no flags	Details Formatted Diff Diff
[Patch] with mac bots fixed (31.94 KB, patch) 2011-11-25 06:31 PST, Pavel Feldman	no flags	Details Formatted Diff Diff
[Patch] rebaselined (31.87 KB, patch) 2011-11-25 07:57 PST, Pavel Feldman	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Pavel Feldman

Comment 1 2011-11-25 05:21:49 PST

Created attachment 116602 [details] Patch

Pavel Feldman

Comment 2 2011-11-25 06:31:33 PST

Created attachment 116614 [details] [Patch] with mac bots fixed

Yury Semikhatsky

Comment 3 2011-11-25 07:07:43 PST

Comment on attachment 116614 [details] [Patch] with mac bots fixed 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()

Pavel Feldman

Comment 4 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.

Pavel Feldman

Comment 5 2011-11-25 07:57:36 PST

Created attachment 116620 [details] [Patch] rebaselined

Pavel Feldman

Comment 6 2011-11-26 07:31:08 PST

Comment on attachment 116620 [details] [Patch] rebaselined Clearing flags on attachment: 116620 Committed r101193: <http://trac.webkit.org/changeset/101193>

Pavel Feldman

Comment 7 2011-11-26 07:31:17 PST

All reviewed patches have been landed. Closing bug.

Pavel Feldman

Comment 8 2011-11-27 07:55:03 PST

Committed r101201: <http://trac.webkit.org/changeset/101201>

Mark Rowe (bdash)

Comment 9 2011-11-28 01:31:05 PST

This has caused crashes after closing any tab with WebKit2: bug 73183.

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