kiosk: Fix kiosk session restart

chrome/browser/chromeos/app_mode/kiosk_crash_restore_browsertest.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/base64.h"
+#include "base/command_line.h"
+#include "base/files/file_util.h"
+#include "base/path_service.h"
+#include "base/run_loop.h"
+#include "base/strings/stringprintf.h"
+#include "chrome/browser/chromeos/app_mode/fake_cws.h"
+#include "chrome/browser/chromeos/app_mode/kiosk_app_manager.h"
+#include "chrome/browser/chromeos/login/test/app_window_waiter.h"
+#include "chrome/browser/chromeos/net/network_portal_detector_test_impl.h"
+#include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos_factory.h"
+#include "chrome/browser/chromeos/policy/device_local_account.h"
+#include "chrome/browser/chromeos/settings/device_settings_cache.h"
+#include "chrome/browser/chromeos/settings/device_settings_test_helper.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/common/chrome_constants.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chromeos/chromeos_paths.h"
+#include "chromeos/chromeos_switches.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/fake_session_manager_client.h"
+#include "chromeos/dbus/fake_shill_manager_client.h"
+#include "extensions/browser/app_window/app_window.h"
+#include "extensions/browser/app_window/app_window_registry.h"
+#include "extensions/browser/app_window/native_app_window.h"
+#include "extensions/test/extension_test_message_listener.h"
+#include "net/dns/mock_host_resolver.h"
+
+namespace em = enterprise_management;
+
+namespace chromeos {
+
+namespace {
+
+const char kTestKioskApp[] = "ggbflgnkafappblpkiflbgpmkfdpnhhe";
+
+void CreateAndInitializeLocalCache() {
+  base::FilePath extension_cache_dir;
+  CHECK(PathService::Get(chromeos::DIR_DEVICE_EXTENSION_LOCAL_CACHE,

[bartfab (slow), 2016/04/07 14:55:41]

Should we not override this path to avoid littering the actual file system with
test files?

[xiyuan, 2016/04/07 21:43:57]

This is overridden under user data dir when not running on the real device.
[1][2] Browser tests uses a ScopeTempDir for user data dir so we will not
pollute the fs.

[1]:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...
[2]: 
https://code.google.com/p/chromium/codesearch#chromium/src/chromeos/chromeos_...

+                         &extension_cache_dir));
+  base::FilePath cache_init_file = extension_cache_dir.Append(
+      extensions::LocalExtensionCache::kCacheReadyFlagFileName);
+  EXPECT_EQ(base::WriteFile(cache_init_file, "", 0), 0);
+}
+
+}  // namespace
+
+class KioskCrashRestoreTest : public InProcessBrowserTest {
+ public:
+  KioskCrashRestoreTest()
+      : owner_key_util_(new ownership::MockOwnerKeyUtil()),
+        fake_cws_(new FakeCWS) {}
+  ~KioskCrashRestoreTest() override {}

[bartfab (slow), 2016/04/07 14:55:42]

Nit: Do you really need to override this?

[xiyuan, 2016/04/07 21:43:56]

Removed.

+
+  // InProcessBrowserTest
+  void SetUp() override {
+    ASSERT_TRUE(embedded_test_server()->InitializeAndListen());
+    InProcessBrowserTest::SetUp();
+  }
+
+  bool SetUpUserDataDirectory() override {
+    SetUpExistingKioskApp();
+    return true;
+  }
+
+  void SetUpInProcessBrowserTestFixture() override {
+    host_resolver()->AddRule("*", "127.0.0.1");
+    SimulateNetworkOnline();
+
+    OverrideDevicePolicy();
+  }
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    const AccountId account_id =
+        AccountId::FromUserEmail(policy::GenerateDeviceLocalAccountUserId(
+            test_app_id_, policy::DeviceLocalAccount::TYPE_KIOSK_APP));
+    const cryptohome::Identification cryptohome_id(account_id);
+
+    command_line->AppendSwitchASCII(switches::kLoginUser, cryptohome_id.id());
+    command_line->AppendSwitchASCII(
+        switches::kLoginProfile,
+        CryptohomeClient::GetStubSanitizedUsername(cryptohome_id));
+
+    fake_cws_->Init(embedded_test_server());
+    fake_cws_->SetUpdateCrx(test_app_id_, test_app_id_ + ".crx", "1.0.0");
+  }
+
+  void SetUpOnMainThread() override {
+    CreateAndInitializeLocalCache();
+
+    embedded_test_server()->StartAcceptingConnections();
+  }
+
+  const std::string& test_app_id() const { return test_app_id_; }
+
+ private:
+  void SetUpExistingKioskApp() {
+    // Create policy data that contains the test app as an existing kiosk app.
+    em::DeviceLocalAccountsProto* device_local_accounts =

[bartfab (slow), 2016/04/07 14:55:41]

Nit: const pointer.

[xiyuan, 2016/04/07 21:43:56]

Done.

+        device_policy_.payload().mutable_device_local_accounts();
+    device_local_accounts->clear_account();

[bartfab (slow), 2016/04/07 14:55:42]

Can you not safely assume that the payload starts out empty?

[xiyuan, 2016/04/07 21:43:56]

Removed.

+
+    em::DeviceLocalAccountInfoProto* account =

[bartfab (slow), 2016/04/07 14:55:42]

Nit: const pointer.

[xiyuan, 2016/04/07 21:43:56]

Done.

+        device_local_accounts->add_account();
+    account->set_account_id(test_app_id_);
+    account->set_type(
+        em::DeviceLocalAccountInfoProto_AccountType_ACCOUNT_TYPE_KIOSK_APP);
+    account->mutable_kiosk_app()->set_app_id(test_app_id_);
+    device_policy_.Build();
+
+    // Store the policy data in device policy cache.
+    em::PolicyData policy_data;
+    CHECK(device_policy_.payload().SerializeToString(
+        policy_data.mutable_policy_value()));
+    const std::string policy_data_string = policy_data.SerializeAsString();
+    std::string encoded;
+    base::Base64Encode(policy_data_string, &encoded);
+
+    const std::string local_state_json = base::StringPrintf(

[bartfab (slow), 2016/04/07 14:55:42]

This looks odd. There are many tests that stub out the policy subsystem very
similarly to the way you do but I do not remember any of them having to inject
policies into the local state file.

[xiyuan, 2016/04/07 21:43:57]

The code that needs to be tested runs early in PreProfileInit. At this time,
policy system has not yet fully loaded. Device policy cache should be populated
in real system and KioskAppManager is initialized with the cached values. The
code here simulates that.

+        "{\"%s\":\"%s\"}", prefs::kDeviceSettingsCache, encoded.c_str());
+
+    base::FilePath local_state_file;
+    CHECK(PathService::Get(chrome::DIR_USER_DATA, &local_state_file));
+    local_state_file = local_state_file.Append(chrome::kLocalStateFilename);
+    base::WriteFile(local_state_file, local_state_json.data(),
+                    local_state_json.size());
+  }
+
+  void SimulateNetworkOnline() {
+    NetworkPortalDetectorTestImpl* network_portal_detector =

[bartfab (slow), 2016/04/07 14:55:42]

Nit: const pointer.

[xiyuan, 2016/04/07 21:43:56]

Done.

+        new NetworkPortalDetectorTestImpl();
+    // Takes ownership of |network_portal_detector|.
+    network_portal_detector::InitializeForTesting(network_portal_detector);
+    network_portal_detector->SetDefaultNetworkForTesting(
+        FakeShillManagerClient::kFakeEthernetNetworkGuid);
+
+    NetworkPortalDetector::CaptivePortalState online_state;
+    online_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE;
+    online_state.response_code = 204;
+    network_portal_detector->SetDetectionResultsForTesting(
+        FakeShillManagerClient::kFakeEthernetNetworkGuid, online_state);
+  }
+
+  void OverrideDevicePolicy() {
+    OwnerSettingsServiceChromeOSFactory::GetInstance()
+        ->SetOwnerKeyUtilForTesting(owner_key_util_);
+    owner_key_util_->SetPublicKeyFromPrivateKey(
+        *device_policy_.GetSigningKey());
+
+    session_manager_client_ = new FakeSessionManagerClient;
+    session_manager_client_->set_device_policy(device_policy_.GetBlob());
+
+    DBusThreadManager::GetSetterForTesting()->SetSessionManagerClient(
+        scoped_ptr<SessionManagerClient>(session_manager_client_));

[bartfab (slow), 2016/04/07 14:55:41]

Nit: s/scoped_ptr/std::unique_ptr/

[xiyuan, 2016/04/07 21:43:57]

Done.

+  }
+
+  std::string test_app_id_ = kTestKioskApp;
+
+  policy::DevicePolicyBuilder device_policy_;
+  scoped_refptr<ownership::MockOwnerKeyUtil> owner_key_util_;
+  FakeSessionManagerClient* session_manager_client_;
+  scoped_ptr<FakeCWS> fake_cws_;

[bartfab (slow), 2016/04/07 14:55:42]

Nit: s/scoped_ptr/std::unique_ptr/

[xiyuan, 2016/04/07 21:43:56]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(KioskCrashRestoreTest);
+};
+
+IN_PROC_BROWSER_TEST_F(KioskCrashRestoreTest, Basic) {
+  ExtensionTestMessageListener launch_data_check_listener(
+      "launchData.isKioskSession = true", false);
+
+  Profile* app_profile = ProfileManager::GetPrimaryUserProfile();

[bartfab (slow), 2016/04/07 14:55:42]

Nit: const pointer.

+  ASSERT_TRUE(app_profile);
+  extensions::AppWindowRegistry* app_window_registry =

[bartfab (slow), 2016/04/07 14:55:41]

Nit: const pointer.

+      extensions::AppWindowRegistry::Get(app_profile);
+  extensions::AppWindow* window =

[bartfab (slow), 2016/04/07 14:55:42]

Nit: const pointer.

+      AppWindowWaiter(app_window_registry, test_app_id()).Wait();
+  EXPECT_TRUE(window);

[bartfab (slow), 2016/04/07 14:55:42]

Nit: s/EXPECT/ASSERT/

[xiyuan, 2016/04/07 21:43:56]

Done.

+
+  window->GetBaseWindow()->Close();
+
+  // Wait until the app terminates if it is still running.
+  if (!app_window_registry->GetAppWindowsForApp(test_app_id()).empty())
+    base::RunLoop().Run();
+
+  EXPECT_TRUE(launch_data_check_listener.was_satisfied());
+}
+
+}  // namespace chromeos