Issue 165173002: Style fixes for demo mode.

Issue 165173002: Style fixes for demo mode. (Closed)

Created:
6 years, 10 months ago by rkc

Modified:
6 years, 9 months ago

Reviewers:
bartfab (slow)

CC:
chromium-reviews, stevenjb+watch_chromium.org, davemoore+watch_chromium.org, oshima+watch_chromium.org, nkostylev+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Style fixes for demo mode. R=bartfab@chromium.org BUG=None. TEST=None. All these style issues are being addressed in https://codereview.chromium.org/205713002/, hence closing this CL.

Patch Set 1 #

Total comments: 4

Created: 6 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+64 lines, -47 lines)			Patch
M	chrome/browser/chromeos/app_mode/kiosk_profile_loader.h	View	2 chunks	+2 lines, -4 lines	0 comments	Download
M	chrome/browser/chromeos/idle_detector.h	View	1 chunk	+4 lines, -0 lines	0 comments	Download
M	chrome/browser/chromeos/idle_detector.cc	View	3 chunks	+5 lines, -6 lines	0 comments	Download
M	chrome/browser/chromeos/login/app_launch_controller.cc	View	2 chunks	+3 lines, -2 lines	0 comments	Download
M	chrome/browser/chromeos/login/authenticator.h	View	1 chunk	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/chromeos/login/demo_mode/demo_app_launcher.h	View	2 chunks	+4 lines, -1 line	1 comment	Download
M	chrome/browser/chromeos/login/demo_mode/demo_app_launcher.cc	View	4 chunks	+13 lines, -8 lines	1 comment	Download
M	chrome/browser/chromeos/login/login_display_host_impl.h	View	2 chunks	+1 line, -1 line	0 comments	Download
M	chrome/browser/chromeos/login/login_display_host_impl.cc	View	2 chunks	+2 lines, -1 line	0 comments	Download
M	chrome/browser/chromeos/login/login_performer.cc	View	1 chunk	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/chromeos/login/mock_authenticator.h	View	1 chunk	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/chromeos/login/mock_authenticator.cc	View	1 chunk	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/chromeos/login/parallel_authenticator.h	View	1 chunk	+6 lines, -4 lines	2 comments	Download
M	chrome/browser/chromeos/login/parallel_authenticator.cc	View	2 chunks	+4 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/webui/chromeos/login/network_screen_handler.h	View	2 chunks	+3 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc	View	5 chunks	+8 lines, -7 lines	0 comments	Download
M	chromeos/chromeos_switches.cc	View	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

bartfab (slow)

6 years, 10 months ago (2014-02-17 15:53:58 UTC) #2

Thanks for uploading the clean-up CL. I left some comments here and replied to a
few others in the original CL.

https://codereview.chromium.org/165173002/diff/1/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/demo_mode/demo_app_launcher.cc (right):

https://codereview.chromium.org/165173002/diff/1/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/demo_mode/demo_app_launcher.cc:47: return (user_id
== kDemoAppUserId);
Nit: The parentheses violate the style guide:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Return_Values

https://codereview.chromium.org/165173002/diff/1/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/demo_mode/demo_app_launcher.h (right):

https://codereview.chromium.org/165173002/diff/1/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/demo_mode/demo_app_launcher.h:15: class Profile;
Nit: |Profile| is only referenced in overridden method declarations. By
convention, there is no need to forward-declare it.

https://codereview.chromium.org/165173002/diff/1/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/parallel_authenticator.h (right):

https://codereview.chromium.org/165173002/diff/1/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/parallel_authenticator.h:136: // success/failure.
If the force_ephemeral flag is true, the crytpohome is
Nit 1: s/force_ephemeral/|force_ephemeral|/
Nit 2: "Flag" usually means command-line flag in Chrome. Better leave out the
term:

"If |force_ephemeral| is true, ..."

https://codereview.chromium.org/165173002/diff/1/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/parallel_authenticator.h:138: // otherwise by the
enterprise policy.
Nit: s/the// ("the" is a definite article but you are not referring to any
specific policy)

Expand Messages | Collapse Messages