chrome/browser/android/vr_shell/ui_unsupported_mode.h - Issue 2902393002: [vr] Bail on unhandled code points.

Side by Side Diff

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

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: chrome/browser/android/vr_shell/ui_unsupported_mode.h

Issue 2902393002: [vr] Bail on unhandled code points. (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:

View unified diff | Download patch

« chrome/browser/android/vr_shell/font_fallback.h ('K') | « chrome/browser/android/vr_shell/ui_scene_manager_unittest.cc ('k') | chrome/browser/android/vr_shell/vr_browser_interface.h » ('j') | chrome/browser/android/vr_shell/vr_shell.cc » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	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

	3 // found in the LICENSE file.

	4

	5 #ifndef CHROME_BROWSER_ANDROID_VR_SHELL_UI_UNSUPPORTED_MODE_H_

	6 #define CHROME_BROWSER_ANDROID_VR_SHELL_UI_UNSUPPORTED_MODE_H_

	7

	8 namespace vr_shell {

	9

	10 // Ensure that this stays in sync with VRUnsupportedMode in enums.xml
	Ilya Sherman 2017/05/26 22:19:51 Please document that this enum is used to back an Please document that this enum is used to back an UMA histogram, and should therefore be treated as append-only. Mark P 2017/05/26 22:29:18 Here's a good comment fragment: // These values ar Show quoted text On 2017/05/26 22:19:51, Ilya Sherman wrote: > Please document that this enum is used to back an UMA histogram, and should > therefore be treated as append-only. Here's a good comment fragment: // These values are written to logs. New enum values can be added, but existing // enums must never be renumbered or deleted and reused. Ian Vollick 2017/05/27 00:02:43 Done. Show quoted text On 2017/05/26 22:19:51, Ilya Sherman wrote: > Please document that this enum is used to back an UMA histogram, and should > therefore be treated as append-only. Done. Ian Vollick 2017/05/27 00:02:43 Sounds great - I've used it directly. Thanks. Show quoted text On 2017/05/26 22:29:18, Mark P wrote: > On 2017/05/26 22:19:51, Ilya Sherman wrote: > > Please document that this enum is used to back an UMA histogram, and should > > therefore be treated as append-only. > > Here's a good comment fragment: > // These values are written to logs. New enum values can be added, but existing > // enums must never be renumbered or deleted and reused. Sounds great - I've used it directly. Thanks.
	11 enum UiUnsupportedMode : int {
	Ilya Sherman 2017/05/26 22:19:51 nit: Mebbe enum class? nit: Mebbe enum class? Ian Vollick 2017/05/27 00:02:43 Done. Show quoted text On 2017/05/26 22:19:51, Ilya Sherman wrote: > nit: Mebbe enum class? Done.
	12 kUnhandledCodePoint = 0,

	13

	14 // This must be last.

	15 kMax,
	Ilya Sherman 2017/05/26 22:19:51 Optional nit: Usually, "max" is used as an alias f Optional nit: Usually, "max" is used as an alias for the last item in an enum, whereas for histograms we really want more of a "count". So, I'd suggest renaming this to "kCount". Ian Vollick 2017/05/27 00:02:43 Done. Show quoted text On 2017/05/26 22:19:51, Ilya Sherman wrote: > Optional nit: Usually, "max" is used as an alias for the last item in an enum, > whereas for histograms we really want more of a "count". So, I'd suggest > renaming this to "kCount". Done.
	16 };

	17

	18 } // namespace vr_shell

	19

	20 #endif // CHROME_BROWSER_ANDROID_VR_SHELL_UI_UNSUPPORTED_MODE_H_

OLD	NEW