[webkit-reviews] review denied: [Bug 199924] Remote WebInspector should enable mock capture devices in UIProcess if doing it in WebProcess : [Attachment 374430] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has denied  review:
Bug 199924: Remote WebInspector should enable mock capture devices in UIProcess
if doing it in WebProcess
https://bugs.webkit.org/show_bug.cgi?id=199924

Attachment 374430: Patch

https://bugs.webkit.org/attachment.cgi?id=374430&action=review




--- Comment #3 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 374430
  --> https://bugs.webkit.org/attachment.cgi?id=374430
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374430&action=review

r-, due to the comment below

> Source/WebKit/UIProcess/WebInspectorProxy.cpp:602
> +    m_inspectedPage->preferences().setMockCaptureDevicesEnabled(enabled);
> +   
MockRealtimeMediaSourceCenter::setMockRealtimeMediaSourceCenterEnabled(enabled)
;

This has a different effect than what `InspectorPageAgent::overrideSetting`
does.  It uses a generated override `Optional<bool>` value that's added to
`WebCore::Settings` to make sure that changing the inspector value doesn't
actually change the underlying setting value.  It's an override on top of the
value.

I think that we should do the same when generating `WebKit::Preferences` as
well (probably by generating something like
`FOR_EACH_WEBKIT_PREFERENCE_WITH_INSPECTOR_OVERRIDE`).	This way, we aren't
overriding the preference's underlying value.

See <https://webkit.org/b/193813> for how it was done for `WebCore::Settings`.

FYI, I'd also have the same sort of logic/thinking as
<https://webkit.org/b/199925> too.