| 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: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Bug Depends on: | 73158, 73188 | ||||||||||
| Bug Blocks: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Pavel Feldman
2011-11-25 05:18:06 PST
Created attachment 116602 [details]
Patch
Created attachment 116614 [details]
[Patch] with mac bots fixed
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() 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. Created attachment 116620 [details]
[Patch] rebaselined
Comment on attachment 116620 [details] [Patch] rebaselined Clearing flags on attachment: 116620 Committed r101193: <http://trac.webkit.org/changeset/101193> All reviewed patches have been landed. Closing bug. Committed r101201: <http://trac.webkit.org/changeset/101201> |