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

Issue 735590 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[DIAL] Consider replacing WeakPtr usage in DialServiceImpl

Project Member Reported by imch...@chromium.org, Jun 21 2017

Issue description

There are a couple of places where WeakPtr is used for DialServiceImpl.

(1) DialSockets are supplied with callbacks bound to WeakPtr of DialServiceImpl. Note that DialServiceImpl owns DialSockets.
(2) On non-ChromeOS, we post a task to a blocking IO thread to call  net::GetNetworkList, with the reply callback bound also using WeakPtr of DialServiceImpl.


For (1) we should be able to use base::Unretained. For (2) we may be able to use a CancelableTaskTracker to replace WeakPtrs.
 

Comment 1 by sko...@chromium.org, Jun 23 2017

Status: Available (was: Untriaged)

Comment 2 by mfo...@chromium.org, Jun 23 2017

Labels: Hotlist-CodeHealth
Owner: imch...@chromium.org
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/878612219980ebb53ba49207d0f12fc52ff443e6

commit 878612219980ebb53ba49207d0f12fc52ff443e6
Author: Derek Cheng <imcheng@chromium.org>
Date: Tue Jul 18 20:44:42 2017

[DIAL] DialServiceImpl / DialSocket cleanups.

- Replace 2 instances of WeakPtr usage in DialServiceImpl with a
CancelableTaskTracker to track thread-hopping calls to obtain network
interfaces.
- Replace the callbacks bound to DialServiceImpl using WeakPtr passed
to DialSocket with a raw pointer DialServiceImpl. This is safe because
DialServiceImpl owns DialSockets.
- Remove WeakPtrFactory from DialServiceImpl.

Bug:  735590 
Change-Id: I4965a33a7a706b057ee7d0f4c5cb6fb98b06dbc2
Reviewed-on: https://chromium-review.googlesource.com/574993
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487595}
[modify] https://crrev.com/878612219980ebb53ba49207d0f12fc52ff443e6/chrome/browser/media/router/discovery/dial/dial_service.cc
[modify] https://crrev.com/878612219980ebb53ba49207d0f12fc52ff443e6/chrome/browser/media/router/discovery/dial/dial_service.h
[modify] https://crrev.com/878612219980ebb53ba49207d0f12fc52ff443e6/chrome/browser/media/router/discovery/dial/dial_service_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment