Integrate Kiosk Mode with enterprise policies.

Integrate Kiosk Mode with enterprise policies.
Get and use the enterprise policies to set the various timeouts for kiosk mode.


R=mnissler@chromium.org,pastarmovj@chromium.org
BUG=chromium-os:27909
TEST=Tested with the test policy server.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=127962

[rkc, 2012-03-16 00:27:32]

On 2012/03/16 00:00:41, Rahul Chaturvedi wrote:

Hi Mattias, Julian,

Could you look through this code to see if it looks right? I'm building a full
new ToT image for ChromeOS and I'll be doing tests on this tomorrow - but I
wanted to send this out to you guys today itself so if you see any obvious
issues, you can call them out before I send this out for final review.

Thanks!

[Mattias Nissler (ping if slow), 2012-03-16 09:56:46]

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc (right):

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc:23: const int
kMinIdleLogoutTimeout = 25000; // ms = 25s.
Where do this values come from? They seem overly restrictive to me. I would have
expected something in the order of max = 10minutes, min = 5seconds.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc:26: const int
kMinIdleLogoutWarningTimeout = 5000; // ms = 5s.
same here

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc:70: int
idle_logout_timeout = 0, idle_logout_warning_timeout = 0;
nit: It's relatively uncommon to have declarations on the same line in the code
base other than for stuff such as x/y or width/height.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc:84:
std::max(idle_logout_timeout, kMinIdleLogoutTimeout));
There's a bug here, as you clobber the value assigned in line 81. A unit test
would catch this.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc:89:
std::max(idle_logout_warning_timeout, kMinIdleLogoutWarningTimeout));
ditto.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc:105: return
base::TimeDelta::FromSeconds(-1);
This seems like a pretty arbitrary value, since it'll translate to an internal
value of -1000000 microseconds. Can we use 0 instead?

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc:115: }
insert blank line.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.h (right):

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.h:29: // code to pull from
the enterprise policy instead of flags and constants.
remove the TODO, as this is exactly what this CL is supposed to achieve AFAICS

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.h:39: // Initialize the
settings helper; this will wait till trust is established
nit: I hope it actually doesn't wait :)

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_idle_logout.cc (right):

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_idle_logout.cc:66:
chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->
use |power_manager| instead of the singleton access boilerplate again?

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_idle_logout.cc:68:
GetIdleLogoutTimeout().InMilliseconds());
nit: +4 spaces indentation

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_idle_logout.cc:89:
GetIdleLogoutTimeout().InMilliseconds());
nit: indentation

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.cc (right):

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.cc:20:
KioskModeScreensaver::KioskModeScreensaver() {
nit could possibly make this more readable by declaring a local for the
KioskModeHelper pointer.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
File chrome/browser/chromeos/ui/screensaver_extension_dialog.cc (right):

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/ui/screensaver_extension_dialog.cc:64:
base::Unretained(this)));
How do you ensure that the dialog object is till alive when the callback fires?

[rkc, 2012-03-19 23:59:48]

Added unit tests, mock class (to be used for browser tests for idle_logout and
screensavers; coming in another CL) and refactored the KioskModeHelper class to
be called KioskModeSettings - since it's more appropriate.

Added xiyuan for the OWNER's reviews for chrome/browser/ui/webui/chromeos

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc (right):

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc:23: const int
kMinIdleLogoutTimeout = 25000; // ms = 25s.
On 2012/03/16 09:56:46, Mattias Nissler wrote:
> Where do this values come from? They seem overly restrictive to me. I would
have
> expected something in the order of max = 10minutes, min = 5seconds.

Done.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc:26: const int
kMinIdleLogoutWarningTimeout = 5000; // ms = 5s.
On 2012/03/16 09:56:46, Mattias Nissler wrote:
> same here

Done.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc:70: int
idle_logout_timeout = 0, idle_logout_warning_timeout = 0;
On 2012/03/16 09:56:46, Mattias Nissler wrote:
> nit: It's relatively uncommon to have declarations on the same line in the
code
> base other than for stuff such as x/y or width/height.

Done.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc:84:
std::max(idle_logout_timeout, kMinIdleLogoutTimeout));
On 2012/03/16 09:56:46, Mattias Nissler wrote:
> There's a bug here, as you clobber the value assigned in line 81. A unit test
> would catch this.

Done and added unit tests to test this and other stuff.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc:89:
std::max(idle_logout_warning_timeout, kMinIdleLogoutWarningTimeout));
On 2012/03/16 09:56:46, Mattias Nissler wrote:
> ditto.

Done.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc:105: return
base::TimeDelta::FromSeconds(-1);
On 2012/03/16 09:56:46, Mattias Nissler wrote:
> This seems like a pretty arbitrary value, since it'll translate to an internal
> value of -1000000 microseconds. Can we use 0 instead?

I specifically wanted a negative value so 0 could be used as a valid value to
indicate no timeout. It shouldn't matter what the magnitude of that negative
value would be.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc:115: }
On 2012/03/16 09:56:46, Mattias Nissler wrote:
> insert blank line.

Done.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.h (right):

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.h:29: // code to pull from
the enterprise policy instead of flags and constants.
On 2012/03/16 09:56:46, Mattias Nissler wrote:
> remove the TODO, as this is exactly what this CL is supposed to achieve AFAICS

Done.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.h:39: // Initialize the
settings helper; this will wait till trust is established
On 2012/03/16 09:56:46, Mattias Nissler wrote:
> nit: I hope it actually doesn't wait :)

Done.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_idle_logout.cc (right):

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_idle_logout.cc:66:
chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->
On 2012/03/16 09:56:46, Mattias Nissler wrote:
> use |power_manager| instead of the singleton access boilerplate again?

Done.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_idle_logout.cc:68:
GetIdleLogoutTimeout().InMilliseconds());
On 2012/03/16 09:56:46, Mattias Nissler wrote:
> nit: +4 spaces indentation

Done.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_idle_logout.cc:89:
GetIdleLogoutTimeout().InMilliseconds());
On 2012/03/16 09:56:46, Mattias Nissler wrote:
> nit: indentation

Done.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.cc (right):

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.cc:20:
KioskModeScreensaver::KioskModeScreensaver() {
On 2012/03/16 09:56:46, Mattias Nissler wrote:
> nit could possibly make this more readable by declaring a local for the
> KioskModeHelper pointer.

Done.

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
File chrome/browser/chromeos/ui/screensaver_extension_dialog.cc (right):

https://chromiumcodereview.appspot.com/9701084/diff/3007/chrome/browser/chrom...
chrome/browser/chromeos/ui/screensaver_extension_dialog.cc:64:
base::Unretained(this)));
On 2012/03/16 09:56:46, Mattias Nissler wrote:
> How do you ensure that the dialog object is till alive when the callback
fires?

g_instance is alive for the lifetime of chrome; so the object should always be
alive.

[xiyuan, 2012-03-20 00:50:16]

chrome/browser/ui/webui/chromeos part LGTM

[Mattias Nissler (ping if slow), 2012-03-20 13:10:35]

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.cc (right):

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.cc:21:
chromeos::KioskModeSettings* kiosk_mode_helper =
variable name should probably change to reflect helper -> settings

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.cc (right):

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.cc:25: KioskModeSettings*
KioskModeSettings::test_instance_ = NULL;
If you were to dependency-inject the KiostModeSettings pointer to classes that
need it on construction time, you wouldn't need this kludge.

Also, I cannot see code that would use the test_instance stuff, and I strongly
suggest you remove it from this CL, as speculatively-added stuff cannot be
reviewed properly anyways.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.cc:54: CrosSettings*
cros_settings = CrosSettings::Get();
You're actually not watching for changes to the settings (they can change at run
time). This might be OK in your case if you can guarantee that the browser
restarts often enough to pick up new settings. Just sayin' so you can consider
it.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.h (right):

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.h:27: const int kFudgeInt
= 100;
what's this? only used in tests, so move there?

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.h:46: virtual bool
IsTestKioskModeEnabled() { return false; }
Why not make the IsKioskModeEnabled() a virtual member then that you can
override?

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc (right):

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc:40:
cros_settings->AddSettingsProvider(device_settings_provider_);  }
closing brace on separate line.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc:58:
base::Unretained(this)));
How about just using base::Closure()?

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc:81:
EXPECT_NE(KioskModeSettings::Get(), (KioskModeSettings*) NULL);
For non-NULL checks, EXPECT_TRUE() works just fine and is used quite a lot in
the code base.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc:92:
EXPECT_TRUE(KioskModeSettings::Get()->GetIdleLogoutTimeout() ==
EXPECT_EQ?

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc:95: // Check
if we go over min.
s/over/under/

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc:99:
EXPECT_TRUE(KioskModeSettings::Get()->GetIdleLogoutTimeout() ==
EXPECT_EQ?

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc:106:
EXPECT_TRUE(KioskModeSettings::Get()->GetIdleLogoutTimeout() ==
EXPECT_EQ? also below...

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
File chrome/browser/chromeos/kiosk_mode/mock_kiosk_mode_settings.h (right):

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/mock_kiosk_mode_settings.h:24: class
MockKioskModeSettings : public KioskModeSettings {
AFAICS, this is not used at all. How about moving it to the CL that actually
needs it?

[Mattias Nissler (ping if slow), 2012-03-20 13:45:30]

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc (right):

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc:58:
base::Unretained(this)));
On 2012/03/20 13:10:35, Mattias Nissler wrote:
> How about just using base::Closure()?

Scratch this, an uninitialized base::Closure() wouldn't work as you do call it.

[rkc, 2012-03-20 21:00:41]

Removed the mock class entirely for now.
I'll re-add it with the BrowserTests that'll be using it.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.cc (right):

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.cc:21:
chromeos::KioskModeSettings* kiosk_mode_helper =
On 2012/03/20 13:10:35, Mattias Nissler wrote:
> variable name should probably change to reflect helper -> settings

Done.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.cc (right):

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.cc:25: KioskModeSettings*
KioskModeSettings::test_instance_ = NULL;
On 2012/03/20 13:10:35, Mattias Nissler wrote:
> If you were to dependency-inject the KiostModeSettings pointer to classes that
> need it on construction time, you wouldn't need this kludge.
> 
> Also, I cannot see code that would use the test_instance stuff, and I strongly
> suggest you remove it from this CL, as speculatively-added stuff cannot be
> reviewed properly anyways.

Removed support code for the mock classes for now.
Done.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.cc:54: CrosSettings*
cros_settings = CrosSettings::Get();
On 2012/03/20 13:10:35, Mattias Nissler wrote:
> You're actually not watching for changes to the settings (they can change at
run
> time). This might be OK in your case if you can guarantee that the browser
> restarts often enough to pick up new settings. Just sayin' so you can consider
> it.

Aren't the kiosk mode settings set in the lockbox specifically to not allow them
to change?

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.h (right):

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.h:27: const int kFudgeInt
= 100;
On 2012/03/20 13:10:35, Mattias Nissler wrote:
> what's this? only used in tests, so move there?

Done.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.h:46: virtual bool
IsTestKioskModeEnabled() { return false; }
On 2012/03/20 13:10:35, Mattias Nissler wrote:
> Why not make the IsKioskModeEnabled() a virtual member then that you can
> override?

Removed all code related to the mock class; I'll put that in a CL that actually
uses the code.
Done.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
File chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc (right):

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc:40:
cros_settings->AddSettingsProvider(device_settings_provider_);  }
On 2012/03/20 13:10:35, Mattias Nissler wrote:
> closing brace on separate line.

Done.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc:81:
EXPECT_NE(KioskModeSettings::Get(), (KioskModeSettings*) NULL);
On 2012/03/20 13:10:35, Mattias Nissler wrote:
> For non-NULL checks, EXPECT_TRUE() works just fine and is used quite a lot in
> the code base.

Done.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc:92:
EXPECT_TRUE(KioskModeSettings::Get()->GetIdleLogoutTimeout() ==
On 2012/03/20 13:10:35, Mattias Nissler wrote:
> EXPECT_EQ?

Done.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc:95: // Check
if we go over min.
On 2012/03/20 13:10:35, Mattias Nissler wrote:
> s/over/under/

Done.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc:99:
EXPECT_TRUE(KioskModeSettings::Get()->GetIdleLogoutTimeout() ==
On 2012/03/20 13:10:35, Mattias Nissler wrote:
> EXPECT_EQ?

Done.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc:106:
EXPECT_TRUE(KioskModeSettings::Get()->GetIdleLogoutTimeout() ==
On 2012/03/20 13:10:35, Mattias Nissler wrote:
> EXPECT_EQ? also below...

Done.

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
File chrome/browser/chromeos/kiosk_mode/mock_kiosk_mode_settings.h (right):

http://codereview.chromium.org/9701084/diff/18002/chrome/browser/chromeos/kio...
chrome/browser/chromeos/kiosk_mode/mock_kiosk_mode_settings.h:24: class
MockKioskModeSettings : public KioskModeSettings {
On 2012/03/20 13:10:35, Mattias Nissler wrote:
> AFAICS, this is not used at all. How about moving it to the CL that actually
> needs it?

Done.

[Mattias Nissler (ping if slow), 2012-03-21 09:52:34]

LGTM!

[commit-bot: I haz the power, 2012-03-21 15:18:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rkc@chromium.org/9701084/38003

[commit-bot: I haz the power, 2012-03-21 15:18:55]

Can't apply patch for file
chrome/browser/chromeos/chrome_browser_main_chromeos.cc.
While running patch -p1 --forward --force;
patching file chrome/browser/chromeos/chrome_browser_main_chromeos.cc
Hunk #1 FAILED at 24.
Hunk #2 succeeded at 169 (offset -37 lines).
Hunk #3 succeeded at 194 (offset -37 lines).
Hunk #4 succeeded at 320 (offset -46 lines).
1 out of 4 hunks FAILED -- saving rejects to file
chrome/browser/chromeos/chrome_browser_main_chromeos.cc.rej

[sky, 2012-03-21 15:36:05]

LGTM