New issue
Advanced search Search tips

Issue 618043 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocked on:
issue 607946

Blocking:
issue 553459



Sign in to add a comment

Do not implicitly create sequences for SequencedTaskRunnerHandle

Project Member Reported by gab@chromium.org, Jun 7 2016

Issue description

In particular don't create a sequence from unsequenced tasks (currently done in SequencedWorkerPool).

Design doc: https://docs.google.com/document/d/1A_LRKyTOCzhRPOY4Q3RsePuw4UCsvxuFYx6D18BaYk0/edit
 

Comment 2 by gab@chromium.org, Jun 7 2016

Note: pinged https://codereview.chromium.org/1414793009/ where SequencedTaskRunner from unsequenced tasks was intentionally implemented in case people on that thread care (it did make sense back then but see design doc above to see why I've changed my mind).
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 22 2016

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

commit 9f79bb50c6a36522eac9e0fb327c644ffb09c405
Author: gab <gab@chromium.org>
Date: Wed Jun 22 22:11:38 2016

WeakPtr requires SequencedTaskRunner, correct interface in blimp_client_session.cc

The caller is already using a SequencedTaskRunner but it's incorrect for this
interface to allow a plain TaskRunner per its use of WeakPtr.

BUG= 618043 

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

[modify] https://crrev.com/9f79bb50c6a36522eac9e0fb327c644ffb09c405/blimp/client/session/blimp_client_session.cc

Comment 4 by gab@chromium.org, Jul 13 2016

Summary: Do not implicitly create sequences for SequencedTaskRunnerHandle (was: Introduce TaskRunnerHandle and deprecate ThreadTaskRunnerHandle/SequencedTaskRunnerHandle.)
Renaming per conclusion of discussion @ https://groups.google.com/a/chromium.org/d/topic/chromium-dev/cd9hldEhIEc/discussion

Comment 5 by gab@chromium.org, Jul 22 2016

Blockedon: 607946
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 28 2016

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

commit 33a2c419dbc05f970bc5de86ad7f3d3919a93bc2
Author: gab <gab@chromium.org>
Date: Thu Jul 28 19:05:07 2016

Revert of Revert "Allow SequencedTaskRunnerHandle::Get() while running unsequenced tasks." (patchset #2 id:40001 of https://codereview.chromium.org/2177373005/ )

Reason for revert:
Actually oops forgot this wasn't supposed to land until this blocker is resolved : https://bugs.chromium.org/p/chromium/issues/detail?id=618043#c5

Original issue's description:
> Revert "Allow SequencedTaskRunnerHandle::Get() while running unsequenced tasks."
>
> This is effectively a manual revert of 5d315e4b4a7cdef34b0c1e863353d03a30781217.
>
> Per the outcome of the discussion on  http://crbug.com/618043  and associated doc/chromium-dev discussion.
>
> BUG= 618043 
>
> Committed: https://crrev.com/41339c7ae8458dd500d1be0e9d4b8da6d8c4c33b
> Cr-Commit-Position: refs/heads/master@{#408440}

TBR=danakj@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 618043 

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

[modify] https://crrev.com/33a2c419dbc05f970bc5de86ad7f3d3919a93bc2/base/threading/sequenced_task_runner_handle.cc
[modify] https://crrev.com/33a2c419dbc05f970bc5de86ad7f3d3919a93bc2/base/threading/sequenced_task_runner_handle.h
[modify] https://crrev.com/33a2c419dbc05f970bc5de86ad7f3d3919a93bc2/base/threading/sequenced_task_runner_handle_unittest.cc
[modify] https://crrev.com/33a2c419dbc05f970bc5de86ad7f3d3919a93bc2/base/threading/sequenced_worker_pool.cc
[modify] https://crrev.com/33a2c419dbc05f970bc5de86ad7f3d3919a93bc2/base/threading/sequenced_worker_pool.h
[modify] https://crrev.com/33a2c419dbc05f970bc5de86ad7f3d3919a93bc2/base/threading/sequenced_worker_pool_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 1 2016

Comment 9 by gab@chromium.org, Aug 1 2016

Status: Fixed (was: Started)

Sign in to add a comment