New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 654176 link

Starred by 1 user
Status: Assigned
Owner:
█ xianglu@chromium.org
Last visit > 30 days ago
Cc:
emir...@chromium.org
mcasas@chromium.org
chfremer@chromium.org
niklase@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Merge VideoCaptureControllerEventHandler into VideoCaptureController

Project Member Reported by mcasas@chromium.org, Oct 8 2016 
VideoCaptureControllerEventHandler is a class (and file)
on its own for no particular reason. It used to be 
VideoCaptureController::EventHandler and was moved 
out with no explanation in [1]. Move it back to be 
VideoCaptureController::Client

Also I'd consider removing VideoCaptureControllerID
and using int32_t, it's silly to opaquify such a 
simple id.


[1] https://codereview.chromium.org/7101001

Comment 1 by chfremer@chromium.org, Oct 10 2016 

There is an advantage of having interfaces live in their own files. If EventHandler lives in the same file as VideoCaptureController it forces all implementations of EventHandler to depend on VideoCaptureController (by including its header file), which they should not need to.

The code review in [1] stated something about "making VS2005 happy" as the purpose for the change. Not sure if that is still needed.

Using the typedef to give a distinct name to the int32_t id type is a valid technique in some code bases, I think. Not sure about Chromium. Personally, I think it is better to use the variable name instead of the type name to express the meaning of the id.

Comment 2 by mcasas@chromium.org, Mar 16 2017

Components: Blink>MediaStream
►

Sign in to add a comment