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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1583233004: Measure time spent during StrictMode violations. (Closed)

Created:
4 years, 11 months ago by Peter Wen
Modified:
4 years, 11 months ago
CC:
chromium-reviews, asvitkine+watch_chromium.org, hartmanng
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Measure time spent during StrictMode violations. BUG=577348, 577185, 508615, 525781, 543201, 556599, 473356 Committed: https://crrev.com/737eedc8a4f880ee598834eef015304e1ea6a440 Cr-Commit-Position: refs/heads/master@{#371267}

Patch Set 1 : ExternalAuthUtils.java #

Patch Set 2 : NotificationUIManager.java #

Patch Set 3 : PathUtils.java #

Patch Set 4 : WebappActivity.java, WebappDirectoryManager.java #

Patch Set 5 : WebappAuthenticator.java #

Patch Set 6 : Fix tests. #

Patch Set 7 : DocumentTabModelImpl.java #

Patch Set 8 : SecureRandomInitializer.java #

Total comments: 9

Patch Set 9 : Fix per review. #

Total comments: 4

Patch Set 10 : Remove WebappAuthenticator.java and ExternalAuthUtils.java. #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+119 lines, -4 lines) Patch
M base/android/java/src/org/chromium/base/PathUtils.java View 1 2 3 chunks +12 lines, -1 line 0 comments Download
M base/android/java/src/org/chromium/base/metrics/RecordHistogram.java View 1 2 3 4 5 9 chunks +18 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java View 1 2 3 chunks +5 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/tabmodel/document/DocumentTabModelImpl.java View 1 2 3 4 5 6 4 chunks +6 lines, -0 lines 1 comment Download
M chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java View 1 2 3 4 5 5 chunks +7 lines, -1 line 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDirectoryManager.java View 1 2 3 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappAuthenticatorTest.java View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappDirectoryManagerTest.java View 1 2 3 4 5 3 chunks +2 lines, -1 line 0 comments Download
M chrome/test/android/javatests/src/org/chromium/chrome/test/MultiActivityTestBase.java View 1 2 3 4 5 5 chunks +5 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 5 6 7 8 9 2 chunks +54 lines, -0 lines 0 comments Download

Dependent Patchsets:

Issue 1618973002 Patch 100001
Issue 1618973002 Patch 120001
Issue 1618973002 Patch 140001

Messages

Total messages: 28 (14 generated)
Peter Wen
+Glenn.
4 years, 11 months ago (2016-01-20 22:14:25 UTC) #5
Peter Wen
Hi Yaron, Finally got through all the tests. Added a shortcut in RecordHistogram that we ...
4 years, 11 months ago (2016-01-21 20:23:18 UTC) #10
Yaron
https://codereview.chromium.org/1583233004/diff/140001/base/android/java/src/org/chromium/base/SecureRandomInitializer.java File base/android/java/src/org/chromium/base/SecureRandomInitializer.java (right): https://codereview.chromium.org/1583233004/diff/140001/base/android/java/src/org/chromium/base/SecureRandomInitializer.java#newcode30 base/android/java/src/org/chromium/base/SecureRandomInitializer.java:30: long time = SystemClock.elapsedRealtime(); I think Ted already fixed ...
4 years, 11 months ago (2016-01-21 20:48:16 UTC) #11
Peter Wen
PTAL, thanks for the quick review! https://codereview.chromium.org/1583233004/diff/140001/base/android/java/src/org/chromium/base/SecureRandomInitializer.java File base/android/java/src/org/chromium/base/SecureRandomInitializer.java (right): https://codereview.chromium.org/1583233004/diff/140001/base/android/java/src/org/chromium/base/SecureRandomInitializer.java#newcode30 base/android/java/src/org/chromium/base/SecureRandomInitializer.java:30: long time = ...
4 years, 11 months ago (2016-01-21 20:59:45 UTC) #12
Yaron
lgtm https://codereview.chromium.org/1583233004/diff/140001/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappAuthenticatorTest.java File chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappAuthenticatorTest.java (right): https://codereview.chromium.org/1583233004/diff/140001/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappAuthenticatorTest.java#newcode16 chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappAuthenticatorTest.java:16: RecordHistogram.disableForTests(); On 2016/01/21 20:59:45, Peter Wen wrote: > ...
4 years, 11 months ago (2016-01-21 21:05:57 UTC) #13
Yaron
not lgtm - we'll need to do those two as a follow up https://codereview.chromium.org/1583233004/diff/160001/chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java File ...
4 years, 11 months ago (2016-01-21 22:03:22 UTC) #14
Peter Wen
PTAL. https://codereview.chromium.org/1583233004/diff/160001/chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java File chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java (right): https://codereview.chromium.org/1583233004/diff/160001/chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java#newcode250 chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java:250: GoogleApiAvailability.getInstance().isGooglePlayServicesAvailable(context); On 2016/01/21 22:03:22, Yaron wrote: > this ...
4 years, 11 months ago (2016-01-21 22:13:54 UTC) #16
Yaron
On 2016/01/21 22:13:54, Peter Wen wrote: > PTAL. > > https://codereview.chromium.org/1583233004/diff/160001/chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java > File > chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java ...
4 years, 11 months ago (2016-01-21 22:25:45 UTC) #17
Peter Wen
+asvitkine@ for histograms.
4 years, 11 months ago (2016-01-21 22:34:17 UTC) #19
Alexei Svitkine (slow)
lgtm
4 years, 11 months ago (2016-01-22 15:45:57 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1583233004/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1583233004/180001
4 years, 11 months ago (2016-01-25 14:08:03 UTC) #22
commit-bot: I haz the power
Committed patchset #10 (id:180001)
4 years, 11 months ago (2016-01-25 18:24:20 UTC) #24
commit-bot: I haz the power
Patchset 10 (id:??) landed as https://crrev.com/737eedc8a4f880ee598834eef015304e1ea6a440 Cr-Commit-Position: refs/heads/master@{#371267}
4 years, 11 months ago (2016-01-25 18:25:42 UTC) #26
Ted C
4 years, 11 months ago (2016-01-26 18:04:31 UTC) #28
Message was sent while issue was closed. 
https://codereview.chromium.org/1583233004/diff/180001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/document/DocumentTabModelImpl.java
(right):

https://codereview.chromium.org/1583233004/diff/180001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/document/DocumentTabModelImpl.java:519:
RecordHistogram.recordTimesHistogram("Android.StrictMode.DocumentTabStateLoad",
This can be hit before native library loaded.

E/AndroidRuntime( 4803): java.lang.UnsatisfiedLinkError: No implementation found
for void
org.chromium.base.metrics.RecordHistogram.nativeRecordCustomTimesHistogramMilliseconds(java.lang.String,
int, long, long, long, int) (tried
Java_org_chromium_base_metrics_RecordHistogram_nativeRecordCustomTimesHistogramMilliseconds
and
Java_org_chromium_base_metrics_RecordHistogram_nativeRecordCustomTimesHistogramMilliseconds__Ljava_lang_String_2IJJJI)
E/AndroidRuntime( 4803): 	at
org.chromium.base.metrics.RecordHistogram.nativeRecordCustomTimesHistogramMilliseconds(Native
Method)
E/AndroidRuntime( 4803): 	at
org.chromium.base.metrics.RecordHistogram.recordCustomTimesHistogramMilliseconds(RecordHistogram.java:196)
E/AndroidRuntime( 4803): 	at
org.chromium.base.metrics.RecordHistogram.recordTimesHistogram(RecordHistogram.java:149)
E/AndroidRuntime( 4803): 	at
org.chromium.chrome.browser.tabmodel.document.DocumentTabModelImpl.startTabStateLoad(DocumentTabModelImpl.java:519)
E/AndroidRuntime( 4803): 	at
org.chromium.chrome.browser.document.DocumentActivity.preInflationStartup(DocumentActivity.java:253)
E/AndroidRuntime( 4803): 	at
org.chromium.chrome.browser.init.ChromeBrowserInitializer.handlePreNativeStartup(ChromeBrowserInitializer.java:114)
E/AndroidRuntime( 4803): 	at
org.chromium.chrome.browser.init.AsyncInitializationActivity.onCreate(AsyncInitializationActivity.java:183)

Powered by Google App Engine
This is Rietveld 408576698