.
Created attachment 383923 [details] Patch
Created attachment 383925 [details] Patch
Comment on attachment 383925 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383925&action=review r=me, overall looks good :) > Source/WebKit/WebProcess/WebPage/RemoteWebInspectorUI.cpp:186 > + // Inspector's diagnostic logging client will never be used unless the page setting is also enabled. I think it's more accurate to say "should never be used". > Source/WebKit/WebProcess/WebPage/RemoteWebInspectorUI.cpp:190 > + m_frontendAPIDispatcher.dispatchCommand("setDiagnosticLoggingAvailable"_s, available); NIT: do you want to use `m_diagnosticLoggingAvailable` instead, as that's this object's actual state, instead of a parameter? > Source/WebKit/WebProcess/WebPage/WebInspectorUI.cpp:306 > + // Inspector's diagnostic logging client will never be used unless the page setting is also enabled. Ditto (RemoteWebInspectorUI.cpp:186) > Source/WebKit/WebProcess/WebPage/WebInspectorUI.cpp:310 > m_frontendAPIDispatcher.dispatchCommand("setDiagnosticLoggingAvailable"_s, available); Ditto (RemoteWebInspectorUI.cpp:190) > Source/WebKit/WebProcess/WebPage/WebInspectorUI.h:87 > +#if ENABLE(INSPECTOR_TELEMETRY) > + void setDiagnosticLoggingAvailable(bool avaliable); > +#endif Why did this move? > Source/WebKit/WebProcess/WebPage/WebInspectorUI.h:160 > + > + Oops?
Comment on attachment 383925 [details] Patch Attachment 383925 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/13268669 New failing tests: fast/dom/domTokenListIterator.html fast/dom/nodeListIterator.html fast/dom/NodeList/nodelist-iterable.html http/tests/security/cross-frame-access-enumeration.html
Created attachment 383950 [details] Archive of layout-test-results from ews212 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews212 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
(In reply to Devin Rousso from comment #3) > Comment on attachment 383925 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=383925&action=review > > r=me, overall looks good :) > > > Source/WebKit/WebProcess/WebPage/RemoteWebInspectorUI.cpp:186 > > + // Inspector's diagnostic logging client will never be used unless the page setting is also enabled. > > I think it's more accurate to say "should never be used". OK > > > Source/WebKit/WebProcess/WebPage/RemoteWebInspectorUI.cpp:190 > > + m_frontendAPIDispatcher.dispatchCommand("setDiagnosticLoggingAvailable"_s, available); > > NIT: do you want to use `m_diagnosticLoggingAvailable` instead, as that's > this object's actual state, instead of a parameter? I never know which is preferable if the state is simply copied. I'll use the member. > > > Source/WebKit/WebProcess/WebPage/WebInspectorUI.cpp:306 > > + // Inspector's diagnostic logging client will never be used unless the page setting is also enabled. > > Ditto (RemoteWebInspectorUI.cpp:186) > > > Source/WebKit/WebProcess/WebPage/WebInspectorUI.cpp:310 > > m_frontendAPIDispatcher.dispatchCommand("setDiagnosticLoggingAvailable"_s, available); > > Ditto (RemoteWebInspectorUI.cpp:190) > > > Source/WebKit/WebProcess/WebPage/WebInspectorUI.h:87 > > +#if ENABLE(INSPECTOR_TELEMETRY) > > + void setDiagnosticLoggingAvailable(bool avaliable); > > +#endif > > Why did this move? Oops forgot to mention in ChangeLog. I mistakenly put it below in the last patch, but it should be in the upper part with other methods that are mainly called via IPC messages. > > > Source/WebKit/WebProcess/WebPage/WebInspectorUI.h:160 > > + > > + > > Oops?
Committed r252702: <https://trac.webkit.org/changeset/252702>
<rdar://problem/57368960>