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

Issue 609742 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

[Telemetry] IsChromeTracingSupported() is called before browser spinning up

Project Member Reported by deanliao@chromium.org, May 6 2016

Issue description

I found that Telemetry's story_runner's _RunStoryAndProcessErrorIfNeeded()'s code block
  try:
    if isinstance(test, story_test.StoryTest):
      test.WillRunStory(state.platform)
    state.WillRunStory(story)

may cause IsChromeTracingSupported() is called before browser spinning up, which makes it returns false, which may raises exception. We should make sure that browser is started before calling IsChromeTracingSupported(). I'm not sure if it is a good idea to change the code to run state.WillRunStory(story) before test.WillRunStory().

Here is the story:

I run ToT Telemetry benchmark smoothness.maps on ChromeOS. It raises exception "Not supported":
./run_benchmark --remote=192.168.0.13 --browser=cros-chrome smoothness.maps -d
…
[ RUN      ] Maps.maps_002
...
Traceback (most recent call last):
  File ".../telemetry/internal/story_runner.py", line 77, in _RunStoryAndProcessErrorIfNeeded
    test.WillRunStory(state.platform)
  File ".../telemetry/web_perf/timeline_based_measurement.py", line 274, in WillRunStory
    raise Exception('Not supported')
Exception: Not supported

The exception is raised because platform.tracing_controller.IsChromeTracingSupported() returns False in WillRunStory() in timeline_based_measurement.py().

However, smoothness.top_25_smooth can run on CrOS. Why? I compare two benchmarks’ running trace and here is the key:

In _RunStoryAndProcessErrorIfNeeded() in story_runner.py:
  try:
    if isinstance(test, story_test.StoryTest):
      test.WillRunStory(state.platform)
    state.WillRunStory(story)

And smoothness.maps’ self.test object (measurement) is TimelineBasedMeasurement (derived from base Benchmark class), which is derived from StoryTest class, so it invokes test.WillRunStory(state.platform). And TBM.WillRunStory() checks platform.tracing_controller.IsChromeTracingSupport() without spinning up CrOS browser first so it doesn’t have tracing backends at that time. Thus TBM.WillRunStory() raises exception.

For smoothness.top_25_smooth, its self.test object is Smoothness, which is derived from LegacyPageTest, which is not derived from StoryTest, so it just calls state.WillRunStory(story). In SharedPageState.WillRunStory(), it doesn’t check IsChromeTracingSupport(), but it spins up browser (self._StartBrowser).  Then after that, state.RunStory(results) is called (in  _RunStoryAndProcessErrorIfNeeded ), and then eventually called IsSupported in chrome_tracing_agent and gets True result.
 
That was intentional for startup tracing against chrome on desktop & android. Does Cros support startup tracing?
So the flow of chrome which supports startup tracing, it calls test.WillRunStory() to start tracing then calls state.WillRunStory() to restart browser if needed, right?

Now the question is: for CrOS chrome, platform.tracing_controller.IsChromeTracingSupported() returns False because it is marked as not support startup tracing and a devtool client instance is not found (because there's no browser instance at that time). 

For such case, is it okay to let state.WillRunStory() runs first to spin up browser then calls test.WillRunStory() start start tracing?


Cc: zh...@chromium.org
Zhenw: can help untangle this. We don't have enough buffer to handle this now, you can wait until Zhen is back.
Labels: -Pri-1 Pri-2
As long as it affects benchmark test cases with self.test derived from story_test.StoryTest (e.g. TimelineBasedMeasurement) and it seems only a few tests use that directly, it is not that urgent. Change to P2.


Status: Assigned (was: Untriaged)
Components: Test>Telemetry
Components: -Tests>Telemetry

Sign in to add a comment