New issue
Advanced search Search tips

Issue 761498 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 689520



Sign in to add a comment

Migrate battor_agent to new TaskScheduler

Project Member Reported by charliea@chromium.org, Sep 1 2017

Issue description

Project Member

Comment 1 by bugdroid1@chromium.org, Sep 6 2017

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

commit 19d407699e8401d5665ec341b79d094533b96eca
Author: Charlie Andrews <charliea@chromium.org>
Date: Wed Sep 06 15:28:37 2017

Migrate BattOrConnectionImpl to new task running architecture

The unit tests already use TestMockTimeTaskRunner, which is compliant
with the new architecture.

Bug:  761498 ,  667892 
Change-Id: I9230055fe2636d3690c88fe961713736e179981b
Reviewed-on: https://chromium-review.googlesource.com/652508
Commit-Queue: Charlie Andrews <charliea@chromium.org>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499967}
[modify] https://crrev.com/19d407699e8401d5665ec341b79d094533b96eca/tools/battor_agent/battor_connection_impl.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 11 2017

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

commit d3c837936aa0bed3e7e522fe0f515db80c46835f
Author: Charlie Andrews <charliea@chromium.org>
Date: Mon Sep 11 18:23:29 2017

Migrate the BattOr agent test to a MockTimeTaskRunner

This is necessary to migrate the BattOr agent to the new Chrome task
running API (http://bit.ly/2jhO3J6). The reason for this is somewhat
subtle: in the old world, we use a "TestSimpleTaskRunner", which runs
delayed tasks in the order in which they're supposed to run but runs
them immediately. Therefore, RunUntilIdle() effectively fasts forwards
through time.

With the new task posting architecture, TestSimpleTaskRunner no longer
does this, instead by default running non-delayed tasks. This means that
when we wait the 100ms after the last clock sync marker in order to
run stop tracing, we're effectively waiting forever instead of what we
previously did, which is immediately assume that 100ms had elapsed.

In order to get around this, I'm migrating the BattOr agent tests to
what they probably should have used in the first place: a
TestMockTimeTaskRunner. This task runner allows us to programmatically
set the current time, which gives us a lot more control in our unit
tests. As part of this, rather than getting our time from
TimeTicks::Now(), we add a layer of indirection, a TickClock, that
can provide the current TimeTicks of the task runnner clock rather than
the real current clock.

Bug:  761498 
Change-Id: I4f59b0bcd7138f11c7b20e659891cbbb066a6b26
Reviewed-on: https://chromium-review.googlesource.com/658259
Reviewed-by: rnephew <rnephew@chromium.org>
Commit-Queue: Charlie Andrews <charliea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500981}
[modify] https://crrev.com/d3c837936aa0bed3e7e522fe0f515db80c46835f/tools/battor_agent/battor_agent.cc
[modify] https://crrev.com/d3c837936aa0bed3e7e522fe0f515db80c46835f/tools/battor_agent/battor_agent.h
[modify] https://crrev.com/d3c837936aa0bed3e7e522fe0f515db80c46835f/tools/battor_agent/battor_agent_unittest.cc

Comment 3 by gab@chromium.org, Sep 13 2017

Blocking: 689520
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 13 2017

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

commit ebe54b346e4538fccaedabebbfeffa1d0f157ad0
Author: Charlie Andrews <charliea@chromium.org>
Date: Wed Sep 13 16:36:54 2017

Migrate the BattOr agent to use sequences instead of thread affinity

This is part of a larger migration described here: http://bit.ly/2jr4bIn

Bug:  667892 ,  761498 
Change-Id: I459de60aedc4ad6bd1cf08177690229644eb1037
Reviewed-on: https://chromium-review.googlesource.com/665164
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Charlie Andrews <charliea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501660}
[modify] https://crrev.com/ebe54b346e4538fccaedabebbfeffa1d0f157ad0/tools/battor_agent/battor_agent.cc
[modify] https://crrev.com/ebe54b346e4538fccaedabebbfeffa1d0f157ad0/tools/battor_agent/battor_agent.h

Status: Fixed (was: Assigned)

Sign in to add a comment