OLD | NEW |
---|---|
(Empty) | |
1 // Copyright 2017 The Chromium Authors. All rights reserved. | |
2 // Use of this source code is governed by a BSD-style license that can be | |
3 // found in the LICENSE file. | |
4 | |
5 #include "chrome/browser/media/router/mojo/media_route_controller.h" | |
6 | |
7 #include <utility> | |
8 | |
9 namespace media_router { | |
10 | |
11 MediaRouteController::Observer::Observer( | |
12 scoped_refptr<MediaRouteController> controller) | |
13 : controller_(std::move(controller)) { | |
imcheng
2017/03/16 20:00:43
No need to move, scoped_refptr supports copying.
dcheng
2017/03/17 06:15:14
This is actually the preferred idiom.
takumif
2017/03/17 21:48:01
To clarify, are you saying that pass-by-value and
| |
14 controller_->AddObserver(this); | |
15 } | |
16 | |
17 MediaRouteController::Observer::~Observer() { | |
18 if (controller_) | |
19 controller_->RemoveObserver(this); | |
20 } | |
21 | |
22 void MediaRouteController::Observer::OnControllerInvalidated() { | |
23 controller_ = nullptr; | |
24 } | |
25 | |
26 MediaRouteController::MediaRouteController( | |
27 MediaRoute::Id route_id, | |
28 mojom::MediaControllerPtr media_controller, | |
29 base::Closure destructor_callback) | |
30 : destructor_callback_(destructor_callback), | |
dcheng
2017/03/17 06:15:14
Pass closures by const ref (you can also pass by v
takumif
2017/03/17 21:48:01
Done.
| |
31 media_controller_(std::move(media_controller)), | |
32 binding_(this), | |
33 route_id_(route_id) {} | |
34 | |
35 void MediaRouteController::Play() { | |
36 media_controller_->Play(); | |
37 } | |
38 | |
39 void MediaRouteController::Pause() { | |
40 media_controller_->Pause(); | |
41 } | |
42 | |
43 void MediaRouteController::Seek(base::TimeDelta time) { | |
44 media_controller_->Seek(time); | |
45 } | |
46 | |
47 void MediaRouteController::SetMute(bool mute) { | |
48 media_controller_->SetMute(mute); | |
49 } | |
50 | |
51 void MediaRouteController::SetVolume(float volume) { | |
52 media_controller_->SetVolume(volume); | |
53 } | |
54 | |
55 void MediaRouteController::OnMediaStatusUpdated(const MediaStatus& status) { | |
56 for (Observer& observer : observers_) | |
57 observer.OnMediaStatusUpdated(status); | |
58 } | |
59 | |
60 void MediaRouteController::OnControllerInvalidated() { | |
61 for (Observer& observer : observers_) | |
62 observer.OnControllerInvalidated(); | |
63 // |this| is deleted here! | |
imcheng
2017/03/16 20:00:43
This may be true, but only if all references were
takumif
2017/03/17 21:48:01
Documenting in the comments that all the reference
imcheng
2017/03/18 00:11:32
Well MediaRouteController is created by MediaRoute
takumif
2017/03/20 22:10:45
I wonder if it is sufficient to document that only
| |
64 } | |
65 | |
66 void MediaRouteController::OnControllerBound() { | |
67 media_controller_.set_connection_error_handler(base::Bind( | |
imcheng
2017/03/16 20:00:43
Can you set this in the ctor? IIRC, you can set th
takumif
2017/03/17 21:48:01
Yes, we can set it in the ctor by enforcing that t
| |
68 &MediaRouteController::OnControllerInvalidated, base::Unretained(this))); | |
69 } | |
70 | |
71 mojom::MediaStatusObserverPtr MediaRouteController::GetObserverPtr() { | |
dcheng
2017/03/17 06:15:14
I would suggest adding this in a followup where it
takumif
2017/03/17 21:48:01
Done.
| |
72 return binding_.CreateInterfacePtrAndBind(); | |
imcheng
2017/03/16 20:00:43
You can also set_connection_error_handler on the b
takumif
2017/03/17 21:48:01
Moving this to CL #2, but yes I think it'd make se
| |
73 } | |
74 | |
75 MediaRouteController::~MediaRouteController() { | |
76 destructor_callback_.Run(); | |
77 } | |
78 | |
79 void MediaRouteController::AddObserver(Observer* observer) { | |
80 observers_.AddObserver(observer); | |
81 } | |
82 | |
83 void MediaRouteController::RemoveObserver(Observer* observer) { | |
84 observers_.RemoveObserver(observer); | |
85 } | |
86 | |
87 } // namespace media_router | |
OLD | NEW |