Fix inspector override conversion in InspectorPageAgent::overrideSetting
<rdar://problem/53419693>
Created attachment 374643 [details] Patch
Comment on attachment 374643 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374643&action=review r=me, nice catch! > Source/WebCore/ChangeLog:15 > + Previously, the conversion for the mock capture value was implicit from a bool pointer to an optional. > + Make an explicit conversion as done for regular settings. Style: there seems to be an extra space before each of these lines. > Source/WebCore/inspector/agents/InspectorPageAgent.cpp:429 > + return *value; Style: I prefer using `.value()` with `WTF::Optional` so it's a bit clearer that it's not a pointer. > Source/WebCore/inspector/agents/InspectorPageAgent.cpp:445 > + auto overrideValue = toOptionalBool(value); NIT: Web Inspector typically uses `as*` instead of `to*` for it's type conversions (see `asBool` in InspectorRuntimeAgent.cpp). > Source/WebCore/inspector/agents/InspectorPageAgent.cpp:448 > + case Inspector::Protocol::Page::Setting::name: { \ NIT: we can remove the `{` and `}` and make this slightly simpler.
Created attachment 374649 [details] Patch for landing
Done. > Style: I prefer using `.value()` with `WTF::Optional` so it's a bit clearer > that it's not a pointer. I like using pointer-like code, for instance "if(value) value->doSomething()" or "if(value) doSomething(*value)". The one case I am sometimes using .value() is when moving the value. "if (value) doSomething(WTFMove(*value))" seems a bit strange.
Comment on attachment 374649 [details] Patch for landing Clearing flags on attachment: 374649 Committed r247707: <https://trac.webkit.org/changeset/247707>
All reviewed patches have been landed. Closing bug.