Speculative patch: Make it safe to delete the TQM inside a Task

Speculative patch: Make it safe to delete the TQM inside a Task

We are seeing null pointer dereferences inside UpdateWorkQueues called
from DoWork. My theory is this is caused by the RenderThreadImpl
getting deleted inside a task which causes the TQM to get deleted while
DoWork is executing.

This patch makes it safe to delete the TQM inside a task.

BUG=474461
Committed: https://crrev.com/18aa2c5d15db2b890fa583a063efcff94a4caa1f
Cr-Commit-Position: refs/heads/master@{#324251}

[alex clarke (OOO till 29th), 2015-04-08 13:00:02]

The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-04-08 13:00:14]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1072473002/1

[alex clarke (OOO till 29th), 2015-04-08 13:00:21]

alexclarke@chromium.org changed reviewers:
+ skyostil@chromium.org

[commit-bot: I haz the power, 2015-04-08 13:11:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-08 13:11:08]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Sami, 2015-04-08 14:24:05]

This looks pretty tidy.

https://codereview.chromium.org/1072473002/diff/1/content/child/scheduler/tas...
File content/child/scheduler/task_queue_manager.cc (right):

https://codereview.chromium.org/1072473002/diff/1/content/child/scheduler/tas...
content/child/scheduler/task_queue_manager.cc:604: // Detect if the
TaskQueueManager got deleted during the
This should probably be moved to within ProcessTaskFromWorkQueue so that we bail
out immediately after the task runs. Actually |protect| could also be moved
there.

https://codereview.chromium.org/1072473002/diff/1/content/child/scheduler/tas...
File content/child/scheduler/task_queue_manager_unittest.cc (right):

https://codereview.chromium.org/1072473002/diff/1/content/child/scheduler/tas...
content/child/scheduler/task_queue_manager_unittest.cc:1044:
TEST_F(TaskQueueManagerTest, DeleteTqmInTask) {
nit: s/Tqm/Manager/ or something since we're not really using that abbreviation
outside comments.

[alex clarke (OOO till 29th), 2015-04-08 14:33:05]

PTAL

https://codereview.chromium.org/1072473002/diff/1/content/child/scheduler/tas...
File content/child/scheduler/task_queue_manager.cc (right):

https://codereview.chromium.org/1072473002/diff/1/content/child/scheduler/tas...
content/child/scheduler/task_queue_manager.cc:604: // Detect if the
TaskQueueManager got deleted during the
On 2015/04/08 14:24:04, Sami wrote:
> This should probably be moved to within ProcessTaskFromWorkQueue so that we
bail
> out immediately after the task runs. Actually |protect| could also be moved
> there.

Done.

https://codereview.chromium.org/1072473002/diff/1/content/child/scheduler/tas...
File content/child/scheduler/task_queue_manager_unittest.cc (right):

https://codereview.chromium.org/1072473002/diff/1/content/child/scheduler/tas...
content/child/scheduler/task_queue_manager_unittest.cc:1044:
TEST_F(TaskQueueManagerTest, DeleteTqmInTask) {
On 2015/04/08 14:24:04, Sami wrote:
> nit: s/Tqm/Manager/ or something since we're not really using that
abbreviation
> outside comments.

Done.

[Sami, 2015-04-08 14:45:56]

Great, lgtm.

https://codereview.chromium.org/1072473002/diff/20001/content/child/scheduler...
File content/child/scheduler/task_queue_manager.cc (right):

https://codereview.chromium.org/1072473002/diff/20001/content/child/scheduler...
content/child/scheduler/task_queue_manager.cc:648: // Detect if the
TaskQueueManager just got deleted.
Could you add a note here that we mustn't access any member variables after this
point?

[alex clarke (OOO till 29th), 2015-04-08 14:59:40]

https://codereview.chromium.org/1072473002/diff/20001/content/child/scheduler...
File content/child/scheduler/task_queue_manager.cc (right):

https://codereview.chromium.org/1072473002/diff/20001/content/child/scheduler...
content/child/scheduler/task_queue_manager.cc:648: // Detect if the
TaskQueueManager just got deleted.
On 2015/04/08 14:45:56, Sami wrote:
> Could you add a note here that we mustn't access any member variables after
this
> point?

Done.

[alex clarke (OOO till 29th), 2015-04-08 14:59:45]

The CQ bit was checked by alexclarke@chromium.org

[alex clarke (OOO till 29th), 2015-04-08 14:59:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
skyostil@chromium.org
Link to the patchset: https://codereview.chromium.org/1072473002/#ps40001
(title: "Added some comments")

[commit-bot: I haz the power, 2015-04-08 15:00:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1072473002/40001

[alex clarke (OOO till 29th), 2015-04-08 16:53:46]

The CQ bit was unchecked by alexclarke@chromium.org

[alex clarke (OOO till 29th), 2015-04-08 16:53:50]

The CQ bit was checked by alexclarke@chromium.org

[commit-bot: I haz the power, 2015-04-08 16:53:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1072473002/40001

[commit-bot: I haz the power, 2015-04-08 17:40:42]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-04-08 17:41:45]

Patchset 3 (id:??) landed as
https://crrev.com/18aa2c5d15db2b890fa583a063efcff94a4caa1f
Cr-Commit-Position: refs/heads/master@{#324251}