[Media Router] Use DialMediaSinkService in MediaRouterMojoImpl

chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/media/router/discovery/dial/dial_media_sink_service_impl.h"
 
 #include "chrome/browser/media/router/discovery/dial/dial_device_data.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/url_request/url_request_context_getter.h"
@@ skipping 16 matching lines @@
 }
 
 void DialMediaSinkServiceImpl::Start() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (dial_registry_)
     return;
 
   dial_registry_ = DialRegistry::GetInstance();
   dial_registry_->RegisterObserver(this);
   dial_registry_->OnListenerAdded();
+  StartTimer();
 }
 
 void DialMediaSinkServiceImpl::Stop() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (!dial_registry_)
     return;
 
   dial_registry_->OnListenerRemoved();
   dial_registry_->UnregisterObserver(this);
   dial_registry_ = nullptr;
+  finish_timer_.reset();

[Kevin M, 2017/05/24 00:52:55]

This is calling unique_ptr::reset() which will destroy the timer object. Do you
mean to call timer->Reset() instead?

[zhaobin, 2017/05/24 17:45:51]

On purpose. Need to destroy timer so that OnDeviceDescriptionAvailable() after
Stop() wont start the timer again.

[Kevin M, 2017/05/24 18:13:29]

OnDeviceDescriptionAvailable() will segfault, though - it dereferences the timer
pointer which was destroyed by Stop().

This line of code resets the unique_ptr. I think you mean to reset the timer
contained by the unique_ptr, so you would use the arrow operator instead:

finish_timer_->Reset();

[zhaobin, 2017/05/25 21:27:06]

No segfault in the latest patch.

 }
 
 DeviceDescriptionService* DialMediaSinkServiceImpl::GetDescriptionService() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (!description_service_.get()) {
     description_service_.reset(new DeviceDescriptionService(
         base::Bind(&DialMediaSinkServiceImpl::OnDeviceDescriptionAvailable,
                    base::Unretained(this)),
         base::Bind(&DialMediaSinkServiceImpl::OnDeviceDescriptionError,
                    base::Unretained(this))));
@@ skipping 16 matching lines @@
   DCHECK(!description_service_);
   description_service_ = std::move(description_service);
 }
 
 void DialMediaSinkServiceImpl::OnDialDeviceEvent(
     const DialRegistry::DeviceList& devices) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DVLOG(2) << "DialMediaSinkServiceImpl::OnDialDeviceEvent found "
            << devices.size() << " devices";
 
-  // Timer starts in |OnDialDeviceEvent()|, and expires 3 seconds later. If
-  // |OnDeviceDescriptionAvailable()| is called after |finish_timer_| expires,
-  // |finish_timer_| is restarted.
-  StartTimer();
-
   current_sinks_.clear();
   current_devices_ = devices;
 
   GetDescriptionService()->GetDeviceDescriptions(devices,
                                                  request_context_.get());
 }
 
 void DialMediaSinkServiceImpl::OnDialError(DialRegistry::DialErrorCode type) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DVLOG(2) << "OnDialError [DialErrorCode]: " << static_cast<int>(type);
@@ skipping 32 matching lines @@
 }
 
 void DialMediaSinkServiceImpl::OnDeviceDescriptionError(
     const DialDeviceData& device,
     const std::string& error_message) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DVLOG(2) << "OnDescriptionFetchesError [message]: " << error_message;
 }
 
 }  // namespace media_router