[webkit-reviews] review denied: [Bug 83557] Web Inspector: Disabling Inspector causes build failure on Windows : [Attachment 136414] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Vivek Galatage
<vivekgalatage at gmail.com>'s request for review:
Bug 83557: Web Inspector: Disabling Inspector causes build failure on Windows
https://bugs.webkit.org/show_bug.cgi?id=83557

Attachment 136414: Patch
https://bugs.webkit.org/attachment.cgi?id=136414&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=136414&action=review


> Source/WebKit/win/WebNodeHighlight.cpp:163
>     
m_inspectedWebView->page()->inspectorController()->drawHighlight(context);

You should wrap m_inspectedWebView in header as well for additional
compile-time checks.

> Source/WebKit/win/WebView.cpp:715
>      if (m_webInspector)

Ditto

> Source/WebKit/win/WebView.cpp:2664
>      m_inspectorClient = new WebInspectorClient(this);

Ditto

> Source/WebKit/win/WebView.cpp:5776
>  HRESULT STDMETHODCALLTYPE WebView::inspector(IWebInspector** inspector)

You probably want to wrap the method instead of its body to make sure call
sites are not accessing it in the disabled inspector mode.

> Tools/WebKitTestRunner/InjectedBundle/LayoutTestController.cpp:461
>  void LayoutTestController::showWebInspector()

Is there a way to hide methods altogether instead of commenting out their
bodies and wrap the call sites? Call site should not mention "inspector" when
ENABLE(INSPECTOR) is off.