Splitting this from other changes to make easy diffing.
Created attachment 261014 [details] Patch
Attachment 261014 [details] did not pass style-queue: ERROR: Source/WebKit/win/WebInspector.cpp:108: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebKit/win/WebInspector.cpp:130: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 2 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 261020 [details] Proposed Fix (fix ios)
Attachment 261020 [details] did not pass style-queue: ERROR: Source/WebKit/win/WebInspector.cpp:108: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebKit/win/WebInspector.cpp:130: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 2 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 261028 [details] Proposed Fix (EWS)
Attachment 261028 [details] did not pass style-queue: ERROR: Source/WebKit/win/WebInspector.cpp:108: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebKit/win/WebInspector.cpp:130: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 2 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 261028 [details] Proposed Fix (EWS) View in context: https://bugs.webkit.org/attachment.cgi?id=261028&action=review r=me > Source/WebCore/WebCore.order:17081 > -__ZN7WebCore20EmptyInspectorClient18inspectorDestroyedEv > +__ZN7WebCore20EmptyInspectorClient22inspectedPageDestroyedEv We typically do not change order files. This seems harmless, but you may want to just not touch this at all. > Source/WebCore/inspector/InspectorFrontendClientLocal.cpp:131 > InspectorFrontendClientLocal::InspectorFrontendClientLocal(InspectorController* inspectedPageController, Page* frontendPage, std::unique_ptr<Settings> settings) > - : m_inspectorController(inspectedPageController) > + : m_inspectedPageController(inspectedPageController) Hmm, I'm not sure about this one. It reads a bit funny. Was there ever a case where "inspectorController" did not mean the WebCore::InspectorController for the inspected page? > Source/WebKit/mac/WebCoreSupport/WebInspectorClient.mm:73 > + WebInspectorFrontendClient *_frontendClient; > + WebInspectorClient *_inspectorClient; Style: These types are not-ObjC so the * should be on the left (on the type). > Source/WebKit/mac/WebCoreSupport/WebInspectorClient.mm:88 > +- (void)setFrontendClient:(WebInspectorFrontendClient *)frontendClient; > +- (void)setInspectorClient:(WebInspectorClient *)inspectorClient; > +- (WebInspectorClient *)inspectorClient; Same with all of these. > Source/WebKit/mac/WebInspector/WebInspector.mm:179 > + // FIXME: this is a really roundabout way to evaluate in the frontend. > + // We could directly use the frontend instead of going through the inspected page. > + if (Page* inspectedPage = core(_inspectedWebView)) > + inspectedPage->inspectorController().evaluateForTestInFrontend(script); Hmm, I think sending to InspectorController is appropriate. Currently it does InspectorController -> InspectorAgent -> event. If there are multiple frontends, this would ensure it goes to all frontends. > Source/WebKit/mac/WebKit.order:1024 > +__ZN18WebInspectorClient22inspectedPageDestroyedEv > +__ZN18WebInspectorClient18closeLocalFrontendEv Ditto: Order files. > Source/WebKit2/mac/WebKit2.order:4328 > +__ZN6WebKit18WebInspectorClient22inspectedPageDestroyedEv > +__ZN6WebKit18WebInspectorClient18closeLocalFrontendEv Ditto: Order files.
Comment on attachment 261028 [details] Proposed Fix (EWS) View in context: https://bugs.webkit.org/attachment.cgi?id=261028&action=review >> Source/WebCore/inspector/InspectorFrontendClientLocal.cpp:131 >> + : m_inspectedPageController(inspectedPageController) > > Hmm, I'm not sure about this one. It reads a bit funny. > > Was there ever a case where "inspectorController" did not mean the WebCore::InspectorController for the inspected page? In the stub frontend, the frontendClient is hooked to the frontend page's inspector controller, and the FrontendChannel is connected to the inspected page's inspector controller. >> Source/WebKit/mac/WebCoreSupport/WebInspectorClient.mm:73 >> + WebInspectorClient *_inspectorClient; > > Style: These types are not-ObjC so the * should be on the left (on the type). Oops, ok. Will double-check style guide and update if necessary. >> Source/WebKit/mac/WebInspector/WebInspector.mm:179 >> + inspectedPage->inspectorController().evaluateForTestInFrontend(script); > > Hmm, I think sending to InspectorController is appropriate. Currently it does InspectorController -> InspectorAgent -> event. If there are multiple frontends, this would ensure it goes to all frontends. Hmm, I suppose that would make sense to ensure it's actually sent over a FrontendChannel. Other code paths for WK2, win, etc do the same thing. I'll remove the comment.
Committed r189654: <http://trac.webkit.org/changeset/189654>