chrome/browser/flag_descriptions.h - Issue 2847823004: [tracing] Allow enabling heap profiling from chrome://flags

Side by Side Diff: chrome/browser/flag_descriptions.h

Issue 2847823004: [tracing] Allow enabling heap profiling from chrome://flags (Closed)

Patch Set: Created 3 years, 7 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

« base/base_switches.h ('K') | « chrome/browser/about_flags.cc ('k') | chrome/browser/flag_descriptions.cc » ('j') | chrome/browser/flag_descriptions.cc » ('J')

OLD	NEW
1 // Copyright 2017 The Chromium Authors. All rights reserved.	1 // Copyright 2017 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 #ifndef CHROME_BROWSER_FLAG_DESCRIPTIONS_H_	5 #ifndef CHROME_BROWSER_FLAG_DESCRIPTIONS_H_

6 #define CHROME_BROWSER_FLAG_DESCRIPTIONS_H_	6 #define CHROME_BROWSER_FLAG_DESCRIPTIONS_H_

7	7

8 // Includes needed for macros allowing conditional compilation of some strings.	8 // Includes needed for macros allowing conditional compilation of some strings.

9 #include "build/build_config.h"	9 #include "build/build_config.h"

10 #include "build/buildflag.h"	10 #include "build/buildflag.h"

(...skipping 3302 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
3313 // Description of the autoplay policy that requires a user gesture on cross	3313 // Description of the autoplay policy that requires a user gesture on cross

3314 // origin iframes.	3314 // origin iframes.

3315 extern const char kAutoplayPolicyCrossOriginUserGestureRequired[];	3315 extern const char kAutoplayPolicyCrossOriginUserGestureRequired[];

3316	3316

3317 // Description of the autoplay policy that has no user gesture requirements.	3317 // Description of the autoplay policy that has no user gesture requirements.

3318 extern const char kAutoplayPolicyNoUserGestureRequired[];	3318 extern const char kAutoplayPolicyNoUserGestureRequired[];

3319	3319

3320 // Description of the autoplay policy that requires a user gesture.	3320 // Description of the autoplay policy that requires a user gesture.

3321 extern const char kAutoplayPolicyUserGestureRequired[];	3321 extern const char kAutoplayPolicyUserGestureRequired[];

3322	3322

	3323 extern const char kEnableHeapProfilingName[];
	Wez 2017/04/28 17:17:12 nit: Can we have a brief comment on this block to nit: Can we have a brief comment on this block to explain these? DmitrySkiba 2017/04/28 19:39:13 Do you really want to add comments like "Name of t Show quoted text On 2017/04/28 17:17:12, Wez wrote: > nit: Can we have a brief comment on this block to explain these? Do you really want to add comments like "Name of the enable heap profiling flag" and "Description of the enable heap profiling flag"? Note that most strings above were moved from xml, where they were mandatory (I think). I think that's enough that this file (and the namespace) is named flag_descriptions. Wez 2017/04/29 01:15:10 The problem is that at-a-glance these look odd - t Show quoted text On 2017/04/28 19:39:13, DmitrySkiba wrote: > On 2017/04/28 17:17:12, Wez wrote: > > nit: Can we have a brief comment on this block to explain these? > > Do you really want to add comments like "Name of the enable heap profiling flag" > and "Description of the enable heap profiling flag"? Note that most strings > above were moved from xml, where they were mandatory (I think). > > I think that's enough that this file (and the namespace) is named > flag_descriptions. The problem is that at-a-glance these look odd - the first two are actually the name & description for the --enable-heap-profiling flag, while the rest are the mode flags. So a pair of comments like: // Name & description for the heap profiling flag. ... // Descriptions of the different heap profiling modes. ... would help clarify why e.g. the first two end in Name and Desription and the rest don't.
	3324 extern const char kEnableHeapProfilingDescription[];

	3325 extern const char kEnableHeapProfilingModePseudo[];
	Wez 2017/04/28 17:17:12 See comment re kEnable... naming for the mode stri See comment re kEnable... naming for the mode strings. These should still end in Description, though. DmitrySkiba 2017/04/28 19:39:13 I'm not sure, consider the full name: flag_descrip Show quoted text On 2017/04/28 17:17:12, Wez wrote: > See comment re kEnable... naming for the mode strings. These should still end in > Description, though. I'm not sure, consider the full name: flag_descriptions::kEnableHeapProfilingModeNative, so it's a description for kEnableHeapProfilingModeNative flag. flag_descriptions::kEnableHeapProfilingModeNativeDescription looks worse IMO. Wez 2017/04/29 01:15:10 Agreed; I was mis-reading the convention based on Show quoted text On 2017/04/28 19:39:13, DmitrySkiba wrote: > On 2017/04/28 17:17:12, Wez wrote: > > See comment re kEnable... naming for the mode strings. These should still end > in > > Description, though. > > I'm not sure, consider the full name: > flag_descriptions::kEnableHeapProfilingModeNative, so it's a description for > kEnableHeapProfilingModeNative flag. > > flag_descriptions::kEnableHeapProfilingModeNativeDescription looks worse IMO. Agreed; I was mis-reading the convention based on kEnable...Description[] above.
	3326 extern const char kEnableHeapProfilingModeNative[];

	3327 extern const char kEnableHeapProfilingTaskProfiler[];

	3328

3323 } // namespace flag_descriptions	3329 } // namespace flag_descriptions

3324	3330

3325 #endif // CHROME_BROWSER_FLAG_DESCRIPTIONS_H_	3331 #endif // CHROME_BROWSER_FLAG_DESCRIPTIONS_H_

OLD	NEW