[webkit-reviews] review granted: [Bug 222888] [GStreamer] CaptureDevice monitor used from UIProcess : [Attachment 425503] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 222888: [GStreamer] CaptureDevice monitor used from UIProcess
https://bugs.webkit.org/show_bug.cgi?id=222888

Attachment 425503: Patch

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




--- Comment #20 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 425503
  --> https://bugs.webkit.org/attachment.cgi?id=425503
Patch

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

> Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.cpp:71
> +}

I would use = default for consistency with above destructor.

> Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.h:64
> +	   virtual void devicesChanged() { }

I would make it virtual pure since there is no point for an observe with
devicesChanged being a no-op

> Source/WebKit/UIProcess/UserMediaProcessManager.h:53
> +    void devicesChanged() final;

Can it be made private?

> Source/WebKit/UIProcess/UserMediaProcessManager.h:63
> +    WebCore::UserMediaClient::DeviceChangeObserverToken
m_deviceChangeObserverToken;

No longer needed?

> Source/WebKit/WebProcess/MediaStream/UserMediaPermissionRequestManager.h:57
> +    class DeviceObserver : public
WebCore::RealtimeMediaSourceCenter::Observer {

Can be made private probably.
You could also make UserMediaPermissionRequestManager a
RealtimeMediaSourceCenter::Observer.

> Source/WebKit/WebProcess/MediaStream/UserMediaPermissionRequestManager.h:59
> +	   DeviceObserver(UserMediaPermissionRequestManager& manager)

explicit.