Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(393)

Issue 1897493002: Even more instrumentation to diagnose an Android render crash. (Closed)

Created:
4 years, 8 months ago by Alexei Svitkine (slow)
Modified:
4 years, 8 months ago
Reviewers:
Nico
CC:
chromium-reviews, asvitkine+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Even more instrumentation to diagnose an Android render crash. We're seeing renderers receiving IPCs to activate a trial to a different state than what was specified on the command line. This CL expands previously added instrumentation to detect if this is happening via separate FieldTrialList instances by also keeping a copy of the seen states between instances. Note: While the locking here is done through the lock on the FieldTrialList instance, this is safe to do because FieldTrialList has a check to ensure there is only a single FieldTrialList instance at a given time, so the single lock is sufficient. BUG=359406 Committed: https://crrev.com/adc6a3f5593b054dcb114d54c8a6486536001e02 Cr-Commit-Position: refs/heads/master@{#387702}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Add more comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+38 lines, -0 lines) Patch
M base/metrics/field_trial.h View 1 chunk +8 lines, -0 lines 0 comments Download
M base/metrics/field_trial.cc View 1 5 chunks +28 lines, -0 lines 0 comments Download
M chrome/browser/chrome_browser_main.cc View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (4 generated)
Alexei Svitkine (slow)
Still chasing that elusive Android renderer crash. :( Nico, mind reviewing?
4 years, 8 months ago (2016-04-15 18:58:44 UTC) #2
Nico
lgtm https://codereview.chromium.org/1897493002/diff/1/base/metrics/field_trial.cc File base/metrics/field_trial.cc (right): https://codereview.chromium.org/1897493002/diff/1/base/metrics/field_trial.cc#newcode644 base/metrics/field_trial.cc:644: if (g_use_global_check_states) { should this block have a ...
4 years, 8 months ago (2016-04-15 19:00:31 UTC) #3
Alexei Svitkine (slow)
https://codereview.chromium.org/1897493002/diff/1/base/metrics/field_trial.cc File base/metrics/field_trial.cc (right): https://codereview.chromium.org/1897493002/diff/1/base/metrics/field_trial.cc#newcode644 base/metrics/field_trial.cc:644: if (g_use_global_check_states) { On 2016/04/15 19:00:31, Nico wrote: > ...
4 years, 8 months ago (2016-04-15 20:28:46 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1897493002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1897493002/20001
4 years, 8 months ago (2016-04-15 20:29:17 UTC) #7
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 8 months ago (2016-04-15 21:33:44 UTC) #8
commit-bot: I haz the power
4 years, 8 months ago (2016-04-15 21:35:29 UTC) #10
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/adc6a3f5593b054dcb114d54c8a6486536001e02
Cr-Commit-Position: refs/heads/master@{#387702}

Powered by Google App Engine
This is Rietveld 408576698