[Webkit-unassigned] [Bug 78575] Web Inspector: Hide dock button when not allowed to dock

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=78575





--- Comment #20 from Pavel Feldman <pfeldman at chromium.org>  2012-03-05 22:25:39 PST ---
(From update of attachment 130252)
View in context: https://bugs.webkit.org/attachment.cgi?id=130252&action=review

> Source/WebCore/inspector/InspectorClient.h:49
> +    virtual void updateDockingAvailability() { }

InspectorClient should not be aware of docking, it is not inspector's business. I'd much rather have ChromeClient::didResiseMainFrame that would convert into what you need in WebKit layer. But I can see that this method allows you getting the information straight to where you need it, so it is probably Ok. I would rename it to didResiseMainFrame though.

> Source/WebCore/inspector/InspectorController.cpp:368
> +    m_inspectorClient->updateDockingAvailability();

The reason I don't like it is that InspectorController represents inspector backend, it should not be chrome-aware. It should not know what dock / undock means.

> Source/WebCore/inspector/InspectorFrontendClient.h:69
> +    virtual bool canAttachWindow() { return true; }

This is what confused me initially: you ask whether you can undock and command that docking is unavailable into the same instance. You call it differently (attach and dock), but this is essentially the same thing. You probably don't want canAttachWindow in this interface.

> Source/WebCore/inspector/InspectorFrontendClientLocal.h:79
> +    virtual bool canAttachWindow();

Why did it get virtual? Probably should not.

> Source/WebCore/inspector/InspectorInstrumentation.cpp:271
> +void InspectorInstrumentation::didResizeMainFrameImpl(Frame* frame)

Inspector instrumentation is only used for the instrumentation (things that get into agents / reported to the front-end).

> Source/WebCore/page/FrameView.cpp:2338
> +                    InspectorInstrumentation::didResizeMainFrame(m_frame.get());

As mentioned above, you should do page->inspectorController()->didResizeMainFrame. In fact, I would even expose InspectorClient on InspectorController and call didResizeMainFrame on the client directly. InspectorClient::didResizeMainFrame.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.