chrome/installer/util/eula_util.h - Issue 12035043: Implementing app command to query EULA acceptance state for Chrome.

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/installer/util/eula_util.h

Issue 12035043: Implementing app command to query EULA acceptance state for Chrome. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: Now using setup.exe; removing all Browser and App Launcher changes. Created 7 years, 11 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 | Annotate | Revision Log

OLD	NEW
(Empty)
	1 // Copyright (c) 2013 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 // This file contains utilities to read and manage EULA for Chrome.
	grt (UTC plus 2) 2013/01/24 02:51:40 i think it's more accurate to say something about i think it's more accurate to say something about the file containing helper functions relating to Chrome's EULA. as it is, the comment implies that there's a utility to read the EULA. huangs 2013/01/24 18:56:03 Done. Show quoted text On 2013/01/24 02:51:40, grt wrote: > i think it's more accurate to say something about the file containing helper > functions relating to Chrome's EULA. as it is, the comment implies that there's > a utility to read the EULA. Done.
	6

	7 #ifndef CHROME_INSTALLER_UTIL_EULA_UTIL_H_

	8 #define CHROME_INSTALLER_UTIL_EULA_UTIL_H_

	9

	10 #include <windows.h>

	11

	12 namespace installer {

	13

	14 // Performs a comprehensive test on EULA acceptance in Chrome.
	grt (UTC plus 2) 2013/01/24 02:51:40 "...in system-level Chrome." "...in system-level Chrome." huangs 2013/01/24 18:56:03 Done (more elaborate comments). Show quoted text On 2013/01/24 02:51:40, grt wrote: > "...in system-level Chrome." Done (more elaborate comments).
	15 // Returns: 0 if EULA not accepted, 1 if EULA accepted, and E_FAIL on error.
	grt (UTC plus 2) 2013/01/24 02:51:40 you're mixing windows HRESULTS with arbitrary valu you're mixing windows HRESULTS with arbitrary values you've picked. please don't do this. either use HRESULTS consistently (e.g., S_FALSE, S_OK, or E_FAIL), or make up your own type. fwiw, i don't see an obvious benefit to using HRESULT here (except if you're coding with malice :-)). using your own enum will add clarity to the code. huangs 2013/01/24 18:56:03 Defined enum EULAAcceptanceResponse. Is it okay t Defined enum EULAAcceptanceResponse. Is it okay that it gets implicitly casted to int, and returned as exit code? grt (UTC plus 2) 2013/01/24 21:04:55 IIRC, a process exit code in Windows is a DWORD, w Show quoted text On 2013/01/24 18:56:03, huangs1 wrote: > Defined enum EULAAcceptanceResponse. Is it okay that it gets implicitly casted > to int, and returned as exit code? IIRC, a process exit code in Windows is a DWORD, which is unsigned. I don't think you should be returning -1 from your process. For consistency with ordinary process exit, I think you should try to make 0 represent the thumbs-up success case, and other non-zero values represent the other cases.
	16 HRESULT IsEULAAccepted();

	17

	18 } // namespace installer

	19

	20 #endif // CHROME_INSTALLER_UTIL_EULA_UTIL_H_

OLD	NEW

« no previous file with comments | « chrome/installer/setup/setup_main.cc ('k') | chrome/installer/util/eula_util.cc » ('j') | chrome/installer/util/eula_util.cc » ('J')