Add a new app API to enable watchdog behavior restarts in kiosk apps

extensions/browser/api/runtime/runtime_apitest.cc

Index: extensions/browser/api/runtime/runtime_apitest.cc
diff --git a/extensions/browser/api/runtime/runtime_apitest.cc b/extensions/browser/api/runtime/runtime_apitest.cc
index 78f5998cffdfeb6d73146667599f22e28ffcdfc1..4585d6368cf054c9522d52ff7623a0df3cca00b3 100644
--- a/extensions/browser/api/runtime/runtime_apitest.cc
+++ b/extensions/browser/api/runtime/runtime_apitest.cc
@@ -11,6 +11,7 @@
 #include "extensions/browser/api/runtime/runtime_api.h"
 #include "extensions/browser/extension_dialog_auto_confirm.h"
 #include "extensions/browser/extension_registry.h"
+#include "extensions/test/extension_test_message_listener.h"
 #include "extensions/test/result_catcher.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 
@@ -128,4 +129,199 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, DISABLED_ChromeRuntimeReload) {
       registry->GetExtensionById(extension_id, ExtensionRegistry::TERMINATED));
 }
 
+namespace {
+
+// An intercepter of the real RuntimeAPIDelegate that simulates a successful
+// restart request every time.
+class RestartOnWatchdogApiDelegate : public RuntimeAPIDelegate {
+ public:
+  // Takes ownership of the |real_api_delegate|.
+  RestartOnWatchdogApiDelegate(
+      std::unique_ptr<RuntimeAPIDelegate> real_api_delegate)
+      : real_api_delegate_(std::move(real_api_delegate)) {}
+  ~RestartOnWatchdogApiDelegate() override {}
+
+  // RuntimeAPIDelegate:
+  void AddUpdateObserver(UpdateObserver* observer) override {
+    real_api_delegate_->AddUpdateObserver(observer);
+  }
+
+  void RemoveUpdateObserver(UpdateObserver* observer) override {
+    real_api_delegate_->RemoveUpdateObserver(observer);
+  }
+
+  base::Version GetPreviousExtensionVersion(
+      const Extension* extension) override {
+    return real_api_delegate_->GetPreviousExtensionVersion(extension);
+  }
+
+  void ReloadExtension(const std::string& extension_id) override {
+    real_api_delegate_->ReloadExtension(extension_id);
+  }
+
+  bool CheckForUpdates(const std::string& extension_id,
+                       const UpdateCheckCallback& callback) override {
+    return real_api_delegate_->CheckForUpdates(extension_id, callback);
+  }
+
+  void OpenURL(const GURL& uninstall_url) override {
+    real_api_delegate_->OpenURL(uninstall_url);
+  }
+
+  bool GetPlatformInfo(api::runtime::PlatformInfo* info) override {
+    return real_api_delegate_->GetPlatformInfo(info);
+  }
+
+  bool RestartDevice(std::string* error_message) override {
+    if (!quit_closure_.is_null()) {
+      quit_closure_.Run();

[Devlin, 2016/05/25 21:53:22]

base::ResetAndReturn(&quit_closure_);

[afakhry, 2016/05/26 00:18:40]

Done.

+      quit_closure_.Reset();
+    }
+
+    *error_message = "Success.";
+    return true;
+  }
+
+  base::TimeTicks WaitForSuccessfulRestart() {
+    base::RunLoop run_loop;
+    quit_closure_ = run_loop.QuitClosure();
+    run_loop.Run();
+    return base::TimeTicks::Now();
+  }
+
+ private:
+  std::unique_ptr<RuntimeAPIDelegate> real_api_delegate_;
+
+  base::Closure quit_closure_;
+
+  DISALLOW_COPY_AND_ASSIGN(RestartOnWatchdogApiDelegate);
+};
+
+}  // namespace
+
+class RestartOnWatchdogApiTest : public ExtensionApiTest {
+ public:
+  RestartOnWatchdogApiTest() {}
+  ~RestartOnWatchdogApiTest() override {}
+
+  void SetUpOnMainThread() override {
+    RuntimeAPI* runtime_api = RuntimeAPI::GetFactoryInstance()->Get(profile());
+    api_delegate_ =
+        new RestartOnWatchdogApiDelegate(std::move(runtime_api->delegate_));
+    runtime_api->delegate_.reset(api_delegate_);
+    runtime_api->SetMinDurationBetweenRestartsForTesting(
+        base::TimeDelta::FromSeconds(3));
+    runtime_api->AllowNonKiostAppsInRestartOnWatchdogForTesting();
+
+    ExtensionApiTest::SetUpOnMainThread();

[Devlin, 2016/05/25 21:53:22]

nit: call this first

[afakhry, 2016/05/26 00:18:39]

Done.

+  }
+
+  base::TimeTicks WaitForSuccessfulRestart() {
+    return api_delegate_->WaitForSuccessfulRestart();
+  }
+
+  bool IsWatchdogTimerRunning() {
+    return RuntimeAPI::GetFactoryInstance()
+        ->Get(profile())
+        ->watchdog_timer_.IsRunning();
+  }
+
+  base::TimeTicks desired_restart_time() {
+    return RuntimeAPI::GetFactoryInstance()
+        ->Get(profile())
+        ->watchdog_timer_.desired_run_time();
+  }
+
+ private:
+  RestartOnWatchdogApiDelegate* api_delegate_;  // Not Owned.
+
+  DISALLOW_COPY_AND_ASSIGN(RestartOnWatchdogApiTest);
+};
+
+IN_PROC_BROWSER_TEST_F(RestartOnWatchdogApiTest, RestartOnWatchdogTest) {

[Devlin, 2016/05/25 21:53:22]

Now that you've simplified some of the tests a bit (yay!), we can actually do
all these in a unittest.  See extension_function_test_utils for how to run a
function without a JS counterpart.  Then we get the advantage of unittest speed
+ not needing to go back and forth between the two test files to understand
what's going on.  (Sorry for not mentioning this earlier - it didn't occur to me
that we really didn't need the JS part here for what we're testing.)

[afakhry, 2016/05/26 00:18:40]

For this tricky and sensitive API, I think we should keep these browser tests
for the following reasons:
- It makes sense to have this test along side the other runtime api tests here.
- Isn't a browsertest is the closest test environment to reality?
- I also found it very convenient modifying the separate JS part and re-run the
test immediately without having to recompile browsertests agin.
- Having the JS part will be helpful to the test team, giving them an example of
how the API is used so that they can add their own tests for the Test Review bug
here: https://bugs.chromium.org/p/chromium/issues/detail?id=607413

[Devlin, 2016/05/27 15:26:36]

Browser tests are orders of magnitude slower and flakier than unit tests. 
Additionally, all of the "tricky and sensitive" aspects of this API are confined
here, in the browser process, which can be in a normal state in a unittest.  The
renderer component is just a vanilla extension API with no added logic (as is
seen here by the lack of renderer/ modified files), and if our extension API
system breaks in the renderer, we'll have hundreds of other browser tests fail. 
Unit tests should almost always be preferred to browser tests, and I don't think
this is an exception.

[afakhry, 2016/06/01 18:44:18]

Done.

+  ASSERT_TRUE(StartEmbeddedTestServer());
+  ExtensionTestMessageListener ready_listener("ready", true);
+  ExtensionTestMessageListener failure_listener("fail", false);
+
+  ASSERT_TRUE(
+      LoadExtension(test_data_dir_.AppendASCII("runtime/restartOnWatchdog")));
+
+  // Run test1.
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ready_listener.Reply("test1");
+  ready_listener.Reset();
+
+  // Run test2.
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ExtensionTestMessageListener request_1_listener("request1", true);
+  base::TimeTicks now = base::TimeTicks::Now();
+  ready_listener.Reply("test2");
+  ready_listener.Reset();
+  ASSERT_TRUE(request_1_listener.WaitUntilSatisfied());
+  ASSERT_TRUE(IsWatchdogTimerRunning());
+  ASSERT_GE(desired_restart_time() - now, base::TimeDelta::FromSeconds(2));
+  request_1_listener.Reply("proceed");
+
+  // Run test3
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ExtensionTestMessageListener request_2_listener("request2", true);
+  now = base::TimeTicks::Now();
+  ready_listener.Reply("test3");
+  ready_listener.Reset();
+  ASSERT_TRUE(request_2_listener.WaitUntilSatisfied());
+  ASSERT_TRUE(IsWatchdogTimerRunning());
+  ASSERT_GE(desired_restart_time() - now, base::TimeDelta::FromSeconds(2));
+  request_2_listener.Reply("proceed");
+
+  // Run test4.
+  // Calls the API with seconds = -1. All restart requests will be cancelled.
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ExtensionTestMessageListener request_3_listener("request3", true);
+  ready_listener.Reply("test4");
+  ready_listener.Reset();
+  ASSERT_TRUE(request_3_listener.WaitUntilSatisfied());
+  ASSERT_FALSE(IsWatchdogTimerRunning());
+  request_3_listener.Reply("proceed");
+
+  // Run test5.
+  // Wait for a successful restart after 3 seconds.
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ExtensionTestMessageListener request_4_listener("request4", true);
+  now = base::TimeTicks::Now();
+  ready_listener.Reply("test5");
+  ready_listener.Reset();
+  ASSERT_TRUE(request_4_listener.WaitUntilSatisfied());
+  ASSERT_TRUE(IsWatchdogTimerRunning());
+  ASSERT_GE(desired_restart_time() - now, base::TimeDelta::FromSeconds(3));
+  base::TimeTicks last_restart_time = WaitForSuccessfulRestart();
+  ASSERT_FALSE(IsWatchdogTimerRunning());
+  ASSERT_GE(base::TimeTicks::Now() - now, base::TimeDelta::FromSeconds(3));
+  request_4_listener.Reply("proceed");
+
+  // Run test6.
+  // This is a restart request that will be throttled, because it happens too
+  // soon after a successful restart.
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ExtensionTestMessageListener request_5_listener("request5", true);
+  ready_listener.Reply("test6");
+  ready_listener.Reset();
+  ASSERT_TRUE(request_5_listener.WaitUntilSatisfied());
+  ASSERT_TRUE(IsWatchdogTimerRunning());
+  // Restart will happen 3 seconds later, even though the request was just one
+  // second.
+  ASSERT_NEAR((desired_restart_time() - last_restart_time).InSecondsF(),
+              base::TimeDelta::FromSeconds(3).InSecondsF(), 0.01);
+  base::TimeTicks this_restart_time = WaitForSuccessfulRestart();
+  ASSERT_FALSE(IsWatchdogTimerRunning());
+  ASSERT_NEAR((this_restart_time - last_restart_time).InSecondsF(),
+              base::TimeDelta::FromSeconds(3).InSecondsF(), 0.01);
+  request_5_listener.Reply("proceed");
+
+  // Succeed.
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ready_listener.Reply("success");
+
+  ASSERT_FALSE(failure_listener.was_satisfied());
+}
+
 }  // namespace extensions