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

Issue 282813002: Show logs in english in TracingController for adb_chrome_profiler (Closed)

Created:
6 years, 7 months ago by JungJik
Modified:
6 years, 7 months ago
CC:
chromium-reviews, klundberg+watch_chromium.org, bulach+watch_chromium.org, yfriedman+watch_chromium.org, dmikurube+memory_chromium.org, ilevy-cc_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Show logs in english in TracingController for adb_chrome_profiler if the target is not in english, adb_profiler_chrome can not know when the profiler is started and finished. because adb_profiler_chrome uses only english regex for parsing the logs. So this is a patch for showing the logs in english, however it still shows toasts by following the target's locale. BUG=373505 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273035

Patch Set 1 : please ignore this #

Patch Set 2 #

Patch Set 3 : please ignore this #

Patch Set 4 : #

Patch Set 5 : fix in TracingControllerAndroid #

Total comments: 2

Patch Set 6 : #

Total comments: 2

Patch Set 7 : #

Total comments: 1

Patch Set 8 : #

Total comments: 1

Patch Set 9 : #

Total comments: 2

Patch Set 10 : #

Total comments: 1

Patch Set 11 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -5 lines) Patch
M content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java View 1 2 3 4 5 6 7 8 9 10 4 chunks +15 lines, -5 lines 0 comments Download

Messages

Total messages: 34 (0 generated)
JungJik
Please PTAL :)
6 years, 7 months ago (2014-05-14 04:45:33 UTC) #1
bulach
thanks Jung Jik! :) hmm... what part of the script is relying on the device ...
6 years, 7 months ago (2014-05-14 10:30:31 UTC) #2
Sami
On 2014/05/14 10:30:31, bulach wrote: > thanks Jung Jik! :) > hmm... what part of ...
6 years, 7 months ago (2014-05-14 12:14:34 UTC) #3
JungJik
On 2014/05/14 12:14:34, Sami wrote: > On 2014/05/14 10:30:31, bulach wrote: > > thanks Jung ...
6 years, 7 months ago (2014-05-14 15:51:40 UTC) #4
Sami
On 2014/05/14 15:51:40, JungJik wrote: > hi, so could I change all toast texts to ...
6 years, 7 months ago (2014-05-15 13:54:17 UTC) #5
JungJik
On 2014/05/15 13:54:17, Sami wrote: > On 2014/05/14 15:51:40, JungJik wrote: > > hi, so ...
6 years, 7 months ago (2014-05-16 04:03:52 UTC) #6
JungJik
On 2014/05/16 04:03:52, JungJik wrote: > On 2014/05/15 13:54:17, Sami wrote: > > On 2014/05/14 ...
6 years, 7 months ago (2014-05-19 10:03:51 UTC) #7
Sami
Thanks for the update. I think this otherwise looks good to go, but I have ...
6 years, 7 months ago (2014-05-19 15:51:19 UTC) #8
JungJik
https://codereview.chromium.org/282813002/diff/70001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java File content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java (right): https://codereview.chromium.org/282813002/diff/70001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java#newcode131 content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java:131: mContext.getResources().updateConfiguration(configuration, displayMetrics); On 2014/05/19 15:51:20, Sami wrote: > Changing ...
6 years, 7 months ago (2014-05-19 16:33:50 UTC) #9
JungJik
On 2014/05/19 16:33:50, JungJik wrote: > https://codereview.chromium.org/282813002/diff/70001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java > File > content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java > (right): > > ...
6 years, 7 months ago (2014-05-22 10:03:05 UTC) #10
Sami
https://codereview.chromium.org/282813002/diff/90001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java File content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java (right): https://codereview.chromium.org/282813002/diff/90001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java#newcode127 content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java:127: Configuration configuration = mContext.getResources().getConfiguration(); Instead of modifying the current ...
6 years, 7 months ago (2014-05-22 10:37:04 UTC) #11
JungJik
On 2014/05/22 10:37:04, Sami wrote: > https://codereview.chromium.org/282813002/diff/90001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java > File > content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java > (right): > > ...
6 years, 7 months ago (2014-05-22 15:08:05 UTC) #12
Sami
Thank you! lgtm with one nit. https://codereview.chromium.org/282813002/diff/110001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java File content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java (right): https://codereview.chromium.org/282813002/diff/110001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java#newcode132 content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java:132: else { Nit: ...
6 years, 7 months ago (2014-05-22 15:09:51 UTC) #13
JungJik
On 2014/05/22 15:09:51, Sami wrote: > Thank you! lgtm with one nit. > > https://codereview.chromium.org/282813002/diff/110001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java ...
6 years, 7 months ago (2014-05-22 16:28:41 UTC) #14
Sami
Thank you. Still lgtm.
6 years, 7 months ago (2014-05-22 16:30:02 UTC) #15
JungJik
The CQ bit was checked by jungjik.lee@samsung.com
6 years, 7 months ago (2014-05-22 16:36:01 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jungjik.lee@samsung.com/282813002/130001
6 years, 7 months ago (2014-05-22 16:38:04 UTC) #17
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are ...
6 years, 7 months ago (2014-05-22 21:03:23 UTC) #18
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-22 21:10:07 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/builds/69337)
6 years, 7 months ago (2014-05-22 21:10:08 UTC) #20
JungJik
On 2014/05/22 21:10:08, I haz the power (commit-bot) wrote: > Try jobs failed on following ...
6 years, 7 months ago (2014-05-23 00:26:46 UTC) #21
Ted C
https://codereview.chromium.org/282813002/diff/130001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java File content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java (right): https://codereview.chromium.org/282813002/diff/130001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java#newcode129 content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java:129: Context context = mContext.createConfigurationContext(configuration); Any idea how long these ...
6 years, 7 months ago (2014-05-23 20:12:54 UTC) #22
JungJik
On 2014/05/23 20:12:54, Ted C wrote: > https://codereview.chromium.org/282813002/diff/130001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java > File > content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java > (right): > ...
6 years, 7 months ago (2014-05-26 04:08:15 UTC) #23
bulach
thanks JungJik! I think I'm with ted here.. :) we could just have a tiny ...
6 years, 7 months ago (2014-05-27 11:07:19 UTC) #24
JungJik
On 2014/05/27 11:07:19, bulach wrote: > thanks JungJik! > I think I'm with ted here.. ...
6 years, 7 months ago (2014-05-27 14:56:52 UTC) #25
bulach
lgtm % small suggestions below, but please ensure the other folks are happy as well ...
6 years, 7 months ago (2014-05-27 15:04:52 UTC) #26
JungJik
On 2014/05/27 15:04:52, bulach wrote: > lgtm % small suggestions below, but please ensure the ...
6 years, 7 months ago (2014-05-27 15:27:11 UTC) #27
Sami
lgtm, happy with this way too. https://codereview.chromium.org/282813002/diff/170001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java File content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java (right): https://codereview.chromium.org/282813002/diff/170001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java#newcode256 content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java:256: // it needs ...
6 years, 7 months ago (2014-05-27 16:01:42 UTC) #28
JungJik
On 2014/05/27 16:01:42, Sami wrote: > lgtm, happy with this way too. > > https://codereview.chromium.org/282813002/diff/170001/content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java ...
6 years, 7 months ago (2014-05-27 17:00:41 UTC) #29
Ted C
lgtm
6 years, 7 months ago (2014-05-27 17:09:50 UTC) #30
JungJik
The CQ bit was checked by jungjik.lee@samsung.com
6 years, 7 months ago (2014-05-27 17:17:10 UTC) #31
JungJik
On 2014/05/27 17:09:50, Ted C wrote: > lgtm Thanks for lgtm :)
6 years, 7 months ago (2014-05-27 17:17:46 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jungjik.lee@samsung.com/282813002/180001
6 years, 7 months ago (2014-05-27 17:18:10 UTC) #33
commit-bot: I haz the power
6 years, 7 months ago (2014-05-27 21:22:24 UTC) #34
Message was sent while issue was closed.
Change committed as 273035

Powered by Google App Engine
This is Rietveld 408576698