New issue
Advanced search Search tips

Issue 729210 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 675631



Sign in to add a comment

//base replacement for content::BrowserThread::DeleteOnThread

Project Member Reported by stanisc@chromium.org, Jun 2 2017

Issue description

Since BrowserThread::FILE is being replaced with sequenced task runners we should probably think about a replacement for DeleteOnThread struct or at least refactoring the code that uses
content::BrowserThread::DeleteOnThread<content::BrowserThread::FILE>
 

Comment 1 by gab@chromium.org, Jun 5 2017

Owner: gab@chromium.org
Status: Started (was: Untriaged)
There is base::OnTaskRunnerDeleter, improving documentation to make this standout more: https://codereview.chromium.org/2921343002
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 7 2017

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

commit 98dee7702162992e0f0a605c09879f0bde1ce987
Author: gab <gab@chromium.org>
Date: Wed Jun 07 15:49:03 2017

Improve documentation and tests for base::OnTaskRunnerDeleter.

And cross-documentation from BrowserThread::DeleteOnThread (ref.  crbug.com/729210 ).

Modernized the tests while trying to make OnTaskRunnerDeleter work for
RefCounted until I realized it wouldn't work as-is, still better than
nothing for now and will help cleaner follow-up.

BUG= 729210 

Review-Url: https://codereview.chromium.org/2921343002
Cr-Commit-Position: refs/heads/master@{#477661}

[modify] https://crrev.com/98dee7702162992e0f0a605c09879f0bde1ce987/base/sequenced_task_runner.h
[modify] https://crrev.com/98dee7702162992e0f0a605c09879f0bde1ce987/base/sequenced_task_runner_unittest.cc
[modify] https://crrev.com/98dee7702162992e0f0a605c09879f0bde1ce987/content/public/browser/browser_thread.h

Comment 3 by gab@chromium.org, Jun 7 2017

Status: Fixed (was: Started)
Highlighted existing replacement in code comments.

Comment 4 by gab@chromium.org, Jun 8 2017

Blocking: 675631
Status: Assigned (was: Fixed)
Summary: //base replacement for content::BrowserThread::DeleteOnThread (was: LL replacement for content::BrowserThread::DeleteOnThread)
Actually I'll keep this opened to track finding a mapping for Browser::DeleteOnThread's usage as a RefCountedTraits.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 8 2017

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

commit 0ad840645e519cfc4ebb9af29ae479e29d296401
Author: gab <gab@chromium.org>
Date: Thu Jun 08 19:12:26 2017

Add cross-reference documentation about the forgotten RefCountedDeleteOnSequence.

BUG= 729210 
TBR=jam@chromium.org, danakj@chromium.org
NOTRY=True

Review-Url: https://codereview.chromium.org/2927993002
Cr-Commit-Position: refs/heads/master@{#478047}

[modify] https://crrev.com/0ad840645e519cfc4ebb9af29ae479e29d296401/base/sequenced_task_runner.h
[modify] https://crrev.com/0ad840645e519cfc4ebb9af29ae479e29d296401/content/public/browser/browser_thread.h

Comment 6 by gab@chromium.org, Jun 8 2017

Status: Fixed (was: Assigned)
Turns out it existed but was well hidden... highlighting it in documentation.

Sign in to add a comment