Add browser tests for enabling Google Drive offline mode on first run.

chrome/browser/chromeos/first_run/drive_first_run_browsertest.cc

Index: chrome/browser/chromeos/first_run/drive_first_run_browsertest.cc
diff --git a/chrome/browser/chromeos/first_run/drive_first_run_browsertest.cc b/chrome/browser/chromeos/first_run/drive_first_run_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..cbcc34b199a73f98e0a6c16003312b280a174b05
--- /dev/null
+++ b/chrome/browser/chromeos/first_run/drive_first_run_browsertest.cc
@@ -0,0 +1,161 @@
+// Copyright 2013 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/bind.h"
+#include "base/files/file_path.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
+#include "base/path_service.h"
+#include "base/strings/string_number_conversions.h"
+#include "chrome/browser/chromeos/first_run/drive_first_run_controller.h"
+#include "chrome/browser/extensions/crx_installer.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/extensions/extension_test_notification_observer.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/test/test_utils.h"
+#include "net/dns/mock_host_resolver.h"
+#include "net/http/http_status_code.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
+
+namespace chromeos {
+
+namespace {
+
+// Directory containing data files for the tests.
+const char kTestDirectory[] = "drive_first_run";
+
+// Directory containing correct hosted app page served by the test server.
+const char kGoodServerDirectory[] = "good";
+
+// Directory containing incorrect hosted app page served by the test server.
+const char kBadServerDirectory[] = "bad";
+
+// Name of the test hosted app .crx file.
+const char kTestAppCrxName[] = "app.crx";
+
+// App id of the test hosted app.
+const char kTestAppId[] = "kipccbklifbfblhpplnmklieangbjnhb";
+
+// The endpoint belonging to the test hosted app.
+const char kTestEndpointUrl[] = "http://example.com/endpoint.html";
+
+}  // namespace
+
+class DriveFirstRunTest : public InProcessBrowserTest,
+                          public DriveFirstRunController::Observer {
+ protected:
+  DriveFirstRunTest() :
+      timed_out_(false),
+      waiting_for_result_(false),
+      success_(false) {}
+
+  virtual void SetUpOnMainThread() OVERRIDE {
+    InProcessBrowserTest::SetUpOnMainThread();

[achuithb, 2013/11/13 04:13:10]

Do you mind pulling out the function bodies to outside the class definition? I
think inlining these makes it harder to read, and I believe the style guide
forbids it except for oneliners.

[Tim Song, 2013/11/13 20:00:25]

Done. There was a discussion on chromium-dev about inlining functions in test
files:
https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/X8D176Uzp6w...

And the style guide has been updated to make an exception for tests if it is
readable.

+    PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir_);
+    test_data_dir_ = test_data_dir_.AppendASCII(kTestDirectory);
+
+    host_resolver()->AddRule("*", "127.0.0.1");
+
+    // |controller_| will delete itself when it completes.
+    controller_ = new DriveFirstRunController();
+    controller_->AddObserver(this);
+    controller_->SetDelaysForTest(0, 10);
+    controller_->SetAppInfoForTest(kTestAppId, kTestEndpointUrl);
+  }
+
+  virtual void CleanUpOnMainThread() OVERRIDE {
+    InProcessBrowserTest::CleanUpOnMainThread();
+    content::RunAllPendingInMessageLoop();
+  }
+
+  void InitTestServer(const std::string& directory) {
+    embedded_test_server()->ServeFilesFromDirectory(
+        test_data_dir_.AppendASCII(directory));
+    ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
+
+    // Configure the endpoint to use the test server's port.
+    const GURL url(kTestEndpointUrl);
+    GURL::Replacements replacements;
+    std::string port(base::IntToString(embedded_test_server()->port()));
+    replacements.SetPortStr(port);
+    endpoint_url_ = url.ReplaceComponents(replacements).spec();
+    controller_->SetAppInfoForTest(kTestAppId, endpoint_url_);
+  }
+
+  void InstallApp() {
+    ExtensionService* extension_service = extensions::ExtensionSystem::Get(
+        browser()->profile())->extension_service();
+    scoped_refptr<extensions::CrxInstaller> installer =
+        extensions::CrxInstaller::CreateSilent(extension_service);
+
+    installer->InstallCrx(test_data_dir_.AppendASCII(kTestAppCrxName));
+    ExtensionTestNotificationObserver observer(browser());
+    observer.WaitForExtensionLoad();
+
+    ASSERT_TRUE(extension_service->GetExtensionById(kTestAppId, false));
+  }
+
+  bool WaitForFirstRunResult() {
+    waiting_for_result_ = true;
+    runner_ = new content::MessageLoopRunner;
+    runner_->Run();
+    EXPECT_FALSE(waiting_for_result_);
+    return success_;
+  }
+
+  // DriveFirstRunController::Observer overrides:
+  virtual void OnCompletion(bool success) OVERRIDE {
+    EXPECT_TRUE(waiting_for_result_);
+    waiting_for_result_ = false;
+    success_ = success;
+    runner_->Quit();

[achuithb, 2013/11/13 04:13:10]

Should you set controller_ to NULL in this function?

[Tim Song, 2013/11/13 20:00:25]

Done.

+  }
+
+  virtual void OnTimedOut() {

[achuithb, 2013/11/13 04:13:10]

OVERRIDE?

[Tim Song, 2013/11/13 20:00:25]

Done.

+    timed_out_ = true;
+  }
+
+  DriveFirstRunController* controller() { return controller_; }

[achuithb, 2013/11/13 04:13:10]

Why do you need this?

[Tim Song, 2013/11/13 20:00:25]

controller_ is private and can't be referenced in the tests.

+  bool timed_out() { return timed_out_; }

[achuithb, 2013/11/13 04:13:10]

Should these functions be const? 

[Tim Song, 2013/11/13 20:00:25]

Done.

+
+ private:
+  // |controller_| is responsible for its own lifetime.
+  DriveFirstRunController* controller_;
+  scoped_refptr<content::MessageLoopRunner> runner_;
+
+  bool timed_out_;
+  bool waiting_for_result_;
+  bool success_;
+  base::FilePath test_data_dir_;
+  std::string endpoint_url_;
+};
+
+IN_PROC_BROWSER_TEST_F(DriveFirstRunTest, OfflineEnabled) {
+  InstallApp();
+  InitTestServer(kGoodServerDirectory);
+  controller()->EnableOfflineMode();
+  EXPECT_TRUE(WaitForFirstRunResult());
+}
+
+IN_PROC_BROWSER_TEST_F(DriveFirstRunTest, AppNotInstalled) {
+  InitTestServer(kGoodServerDirectory);
+  controller()->EnableOfflineMode();
+  EXPECT_FALSE(WaitForFirstRunResult());
+  EXPECT_FALSE(timed_out());
+}
+
+IN_PROC_BROWSER_TEST_F(DriveFirstRunTest, TimedOut) {
+  InstallApp();
+  InitTestServer(kBadServerDirectory);
+  controller()->SetDelaysForTest(0, 0);

[achuithb, 2013/11/13 04:13:10]

Won't the test pass even with a good directory if the timeout is 0?

[Tim Song, 2013/11/13 20:00:25]

That's true, but it would take a few seconds to complete the flow successfully.
I'm not sure it's a good idea to wait that long in a browser test in the
expected case. This test case mainly tests that we get an expected time out and
not a test timeout that would cause an unexpected exit.

[achuithb, 2013/11/13 23:11:54]

Could you please add a comment explaining what this test is doing?

[Tim Song, 2013/11/14 01:18:49]

Done.

+  controller()->EnableOfflineMode();
+  EXPECT_FALSE(WaitForFirstRunResult());
+  EXPECT_TRUE(timed_out());
+}
+
+}  // namespace chromeos