Bug 161976 - [MediaStream] Minor cleanup
Summary: [MediaStream] Minor cleanup
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
Keywords: InRadar
Depends on:
Reported: 2016-09-14 10:15 PDT by Eric Carlson
Modified: 2016-09-16 11:51 PDT (History)
5 users (show)

See Also:

Proposed patch. (97.75 KB, patch)
2016-09-14 10:27 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Proposed patch. (97.75 KB, patch)
2016-09-14 11:00 PDT, Eric Carlson
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2016-09-14 10:15:37 PDT
- MediaStreamTrackSourcesCallback and SourceInfo aren't used, remove them
- CaptureDeviceManager is used in platform, move them there
- Getting the media device list doesn't need to be asynchronous
Comment 1 Radar WebKit Bug Importer 2016-09-14 10:17:32 PDT
Comment 2 Eric Carlson 2016-09-14 10:27:46 PDT
Created attachment 288828 [details]
Proposed patch.
Comment 3 Eric Carlson 2016-09-14 11:00:06 PDT
Created attachment 288831 [details]
Proposed patch.
Comment 4 youenn fablet 2016-09-14 12:21:09 PDT
Comment on attachment 288831 [details]
Proposed patch.

So the major point here is to make the retrieval of devices synchronous.
That seems to make sense to me. Maybe this change should be added to the ChangeLog.

I'll finish the review tomorrow but this looks good to me.
The win bot might want some changes to DerivedSources.cpp.

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

> Source/WebCore/platform/mediastream/CaptureDevice.h:37
> +    enum SourceKind { Audio, Video };

class enum?

> Source/WebCore/platform/mediastream/CaptureDeviceManager.cpp:2
> + * Copyright (C) 2015 Apple Inc. All rights reserved.

Updating the copyright?

> Source/WebCore/platform/mediastream/CaptureDeviceManager.cpp:45
> +}

In the header?

> Source/WebCore/platform/mediastream/CaptureDeviceManager.h:27
> +#define CaptureDeviceManager_h


> Source/WebCore/platform/mediastream/CaptureDeviceManager.h:34
> +#include <wtf/text/WTFString.h>

Probably no need for the last two includes

> Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:44
>  #include "RealtimeMediaSource.h"

Probably this include is not needed
Comment 5 Eric Carlson 2016-09-14 14:00:46 PDT
Committed r205929: https://trac.webkit.org/r205929
Comment 6 Alex Christensen 2016-09-16 09:24:21 PDT
Comment 7 Alex Christensen 2016-09-16 11:51:58 PDT