Commit 5418223b authored by Bin Zhao's avatar Bin Zhao Committed by Commit Bot

[Dial] Destroy DialAPI class on the UI thread

I do not have a local repro... As Derek mentioned, DialAPI is destroyed on the IO thread, but somehow referenced after deletion on the UI thread. Not quite sure how that could happen since
it is scoped_refptr<DialAPI>.

It might relate to delete RefCountedKeyedService on the IO thread. Tentatively fix it by destroying DialAPI instance on the UI thread.

BUG=740489

Change-Id: I3e638c14a77ffa9c5726519571d25874a20cd5f0
Reviewed-on: https://chromium-review.googlesource.com/567025
Commit-Queue: Bin Zhao <zhaobin@chromium.org>
Reviewed-by: default avatarmark a. foltz <mfoltz@chromium.org>
Reviewed-by: default avatarDerek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491241}
parent ead6bdb4
...@@ -34,18 +34,19 @@ DialAPI::DialAPI(Profile* profile) ...@@ -34,18 +34,19 @@ DialAPI::DialAPI(Profile* profile)
: RefcountedKeyedService( : RefcountedKeyedService(
BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)), BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)),
profile_(profile), profile_(profile),
dial_registry_(nullptr) { dial_registry_(nullptr),
num_on_device_list_listeners_(0) {
EventRouter::Get(profile)->RegisterObserver( EventRouter::Get(profile)->RegisterObserver(
this, api::dial::OnDeviceList::kEventName); this, api::dial::OnDeviceList::kEventName);
} }
DialAPI::~DialAPI() { DialAPI::~DialAPI() {
// TODO(zhaobin): Call dial_registry_->UnregisterObserver() instead. In if (!dial_registry_)
// current implementation, UnregistryObserver() does not StopDiscovery() and return;
// causes crash in ~DialRegistry(). May keep a listener count and
// Register/UnregisterObserver as needed. // Remove pending listeners from dial registry.
if (dial_registry_) for (int i = 0; i < num_on_device_list_listeners_; i++)
dial_registry_->StopPeriodicDiscovery(); dial_registry_->OnListenerRemoved();
} }
DialRegistry* DialAPI::dial_registry() { DialRegistry* DialAPI::dial_registry() {
...@@ -77,12 +78,14 @@ void DialAPI::OnListenerRemoved(const EventListenerInfo& details) { ...@@ -77,12 +78,14 @@ void DialAPI::OnListenerRemoved(const EventListenerInfo& details) {
void DialAPI::NotifyListenerAddedOnIOThread() { void DialAPI::NotifyListenerAddedOnIOThread() {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
VLOG(2) << "DIAL device event listener added."; VLOG(2) << "DIAL device event listener added.";
++num_on_device_list_listeners_;
dial_registry()->OnListenerAdded(); dial_registry()->OnListenerAdded();
} }
void DialAPI::NotifyListenerRemovedOnIOThread() { void DialAPI::NotifyListenerRemovedOnIOThread() {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
VLOG(2) << "DIAL device event listener removed"; VLOG(2) << "DIAL device event listener removed";
--num_on_device_list_listeners_;
dial_registry()->OnListenerRemoved(); dial_registry()->OnListenerRemoved();
} }
...@@ -161,7 +164,17 @@ void DialAPI::SendErrorOnUIThread(const DialRegistry::DialErrorCode code) { ...@@ -161,7 +164,17 @@ void DialAPI::SendErrorOnUIThread(const DialRegistry::DialErrorCode code) {
EventRouter::Get(profile_)->BroadcastEvent(std::move(event)); EventRouter::Get(profile_)->BroadcastEvent(std::move(event));
} }
void DialAPI::ShutdownOnUIThread() {} void DialAPI::ShutdownOnUIThread() {
EventRouter::Get(profile_)->UnregisterObserver(this);
if (!dial_registry_)
return;
BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
base::BindOnce(&DialRegistry::UnregisterObserver,
base::Unretained(dial_registry_),
base::RetainedRef(this)));
}
void DialAPI::SetDeviceForTest( void DialAPI::SetDeviceForTest(
const media_router::DialDeviceData& device_data, const media_router::DialDeviceData& device_data,
......
...@@ -96,6 +96,9 @@ class DialAPI : public RefcountedKeyedService, ...@@ -96,6 +96,9 @@ class DialAPI : public RefcountedKeyedService,
// |dial_registry_|. // |dial_registry_|.
media_router::DialRegistry* dial_registry_; media_router::DialRegistry* dial_registry_;
// Number of dial.onDeviceList event listeners.
int num_on_device_list_listeners_;
// Device data for testing. // Device data for testing.
std::unique_ptr<media_router::DialDeviceData> test_device_data_; std::unique_ptr<media_router::DialDeviceData> test_device_data_;
std::unique_ptr<media_router::DialDeviceDescriptionData> std::unique_ptr<media_router::DialDeviceDescriptionData>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment