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

extensions/browser/api/runtime/runtime_api.cc

Index: extensions/browser/api/runtime/runtime_api.cc
diff --git a/extensions/browser/api/runtime/runtime_api.cc b/extensions/browser/api/runtime/runtime_api.cc
index e62805e674289632f33a1786b7a89eac8b06d478..ef5202e1ad1c854cc7f8d8f03573d6af2fe0fd2f 100644
--- a/extensions/browser/api/runtime/runtime_api.cc
+++ b/extensions/browser/api/runtime/runtime_api.cc
@@ -10,6 +10,7 @@
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "base/version.h"
 #include "content/public/browser/browser_context.h"
@@ -76,6 +77,12 @@ const char kPrefPreviousVersion[] = "previous_version";
 // with the equivalent Pepper API.
 const char kPackageDirectoryPath[] = "crxfs";
 
+// Preferece keys for storing the last successful restart on watchdog requests.
+const char kPrefRestartOnWatchdogTime[] = "restart_on_watchdog_time_prefs";
+const char kPrefLastRestartTime[] = "last_restart_on_watchdog_time";
+
+const int kMinDurationBetweenSuccessiveRestartsHours = 3;
+
 void DispatchOnStartupEventImpl(BrowserContext* browser_context,
                                 const std::string& extension_id,
                                 bool first_call,
@@ -156,7 +163,9 @@ RuntimeAPI::RuntimeAPI(content::BrowserContext* context)
     : browser_context_(context),
       dispatch_chrome_updated_event_(false),
       extension_registry_observer_(this),
-      process_manager_observer_(this) {
+      process_manager_observer_(this),
+      minimum_duration_between_restarts_(base::TimeDelta::FromHours(
+          kMinDurationBetweenSuccessiveRestartsHours)) {
   // RuntimeAPI is redirected in incognito, so |browser_context_| is never
   // incognito.
   DCHECK(!browser_context_->IsOffTheRecord());
@@ -321,10 +330,66 @@ bool RuntimeAPI::RestartDevice(std::string* error_message) {
   return delegate_->RestartDevice(error_message);
 }
 
+void RuntimeAPI::RestartDeviceOnWatchdogTimeout(
+    const Extension* extension,
+    int seconds,
+    const OnWatchdogTimeoutCallback& callback) {
+  if (seconds == -1) {
+    watchdog_timer_.Stop();
+    callback.Run(false, "Restart on watchdog was canceled.");
+    return;
+  }
+
+  watchdog_timer_.Stop();

[xiyuan, 2016/05/13 17:56:15]

|watchdog_timer_| is a per-profile resource. We probably should not allow
multiple extensions to use it at the same time. If it is set by extension A, we
should not allow extension B to mess with it. That is, the first extension grabs
the resource and other extensions should get an error.

[afakhry, 2016/05/14 01:27:35]

Done.

+  watchdog_timer_.Start(
+      FROM_HERE, base::TimeDelta::FromSeconds(seconds),
+      base::Bind(&RuntimeAPI::OnRestartWatchdogTimeout, base::Unretained(this),

[xiyuan, 2016/05/13 17:56:15]

nit: There is another Start on OneShotTimer interface that we can pass the
|this| as receive and avoid using base::Unretained if the OneShotTimer is a
member of |this|.

[afakhry, 2016/05/14 01:27:35]

Yes, the problem with that interface is that I won't be able to bind |extension|
and |callback| to the created callback. So I had to use a manual Bind().

[xiyuan, 2016/05/16 18:25:17]

Acknowledged.

+                 extension, callback));
+}
+
 bool RuntimeAPI::OpenOptionsPage(const Extension* extension) {
   return delegate_->OpenOptionsPage(extension);
 }
 
+void RuntimeAPI::OnRestartWatchdogTimeout(
+    const Extension* extension,
+    const OnWatchdogTimeoutCallback& callback) {
+  // Throttle restart requests that are received too soon successively.
+  base::TimeTicks now = base::TimeTicks::Now();

[xiyuan, 2016/05/13 17:56:15]

Time is always a trick topic. base::TimeTicks is a tick count and makes no sense
when system reboots [1]. We cannot store that and compare against it. 

We have to use wall clock base::Time for this. We need to be careful since the
wall clock could be skewed and changed. We need to filter out insane values such
as long in the past or in the future. And document the behavior in API json.

[1] https://www.chromium.org/developers/design-documents/sane-time

[afakhry, 2016/05/14 01:27:35]

Thanks for providing the link. Changed it to base::Time.

+  ExtensionPrefs* prefs = ExtensionPrefs::Get(browser_context_);

[xiyuan, 2016/05/13 17:56:15]

Sorry for flipping on my suggestion. Let's store the reboot record in
extension's state store. See AlarmManager::WriteToStorage and its
OnExtensionLoaded.

[afakhry, 2016/05/14 01:27:35]

Done.

+  DCHECK(prefs);
+
+  const base::DictionaryValue* restart_on_watchdog_prefs = nullptr;
+  if (prefs->ReadPrefAsDictionary(extension->id(), kPrefRestartOnWatchdogTime,
+                                  &restart_on_watchdog_prefs)) {
+    double last_restart_ticks = 0.0;
+    if (restart_on_watchdog_prefs->GetDouble(kPrefLastRestartTime,
+                                             &last_restart_ticks)) {
+      base::TimeTicks last_time = base::TimeTicks::FromInternalValue(
+          static_cast<int64_t>(last_restart_ticks));
+      if ((now - last_time) < minimum_duration_between_restarts_) {
+        callback.Run(false, "Error: Called too soon since the last restart.");
+        // TODO(afakhry): Figure out what to do here. Do we restart later?

[xiyuan, 2016/05/13 17:56:15]

We should still reboot but after the minimum reboot time is satisfied. Propose
to derive a timeout before we set the |watchdog_timer_|, where we take the
function's input and use our last reboot record to extend that value to avoid
rebooting too fast.

[afakhry, 2016/05/14 01:27:35]

Done. Rebooting after the minimum reboot time has passed.

+        return;
+      }
+    }
+  }
+
+  std::string error_message;
+  const bool success = delegate_->RestartDevice(&error_message);
+
+  if (success) {
+    // Store |now| as the last successful restart request time.
+    base::DictionaryValue* prefs_to_update = new base::DictionaryValue();
+    prefs_to_update->SetDouble(kPrefLastRestartTime,
+                               static_cast<double>(now.ToInternalValue()));
+    prefs->UpdateExtensionPref(extension->id(), kPrefRestartOnWatchdogTime,
+                               prefs_to_update);
+  }
+
+  callback.Run(success, error_message);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 // static
@@ -551,6 +616,37 @@ ExtensionFunction::ResponseAction RuntimeRestartFunction::Run() {
   return RespondNow(NoArguments());
 }
 
+ExtensionFunction::ResponseAction RuntimeRestartOnWatchdogFunction::Run() {
+  std::unique_ptr<api::runtime::RestartOnWatchdog::Params> params(
+      api::runtime::RestartOnWatchdog::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());

[xiyuan, 2016/05/13 17:56:15]

Check ExtensionsBrowserClient::Get()->IsRunningInForcedAppMode() and fail if not
in kiosk mode.

[afakhry, 2016/05/14 01:27:35]

I added a TODO here instead, because I'll need more time to think about how to
simulate that in the browser tests that I added.

[afakhry, 2016/05/17 18:38:31]

Now Done.

+  int seconds = params->seconds;
+
+  if (seconds < -1) {
+    return RespondNow(
+        Error("Invalid argument: *.", base::IntToString(seconds)));
+  }
+
+  RuntimeAPI::GetFactoryInstance()
+      ->Get(browser_context())
+      ->RestartDeviceOnWatchdogTimeout(
+          extension(), seconds,
+          base::Bind(&RuntimeRestartOnWatchdogFunction::OnWatchdogTimeout,
+                     this));
+
+  return RespondLater();
+}
+
+void RuntimeRestartOnWatchdogFunction::OnWatchdogTimeout(
+    bool success,
+    const std::string& message) {
+  if (!success)
+    WriteToConsole(content::CONSOLE_MESSAGE_LEVEL_ERROR, message);
+
+  Respond(ArgumentList(

[xiyuan, 2016/05/13 17:56:15]

I thought we would invoke the callback when the watchdog timer is set. Maybe we
should clarify with Matt of what he expects?

If we response when the timer fires, we need to clean up the pending calls on
subsequent calls that cancels previous timeout. Otherwise, we leave a leak in
the renderer.

[afakhry, 2016/05/14 01:27:35]

Done. I made sure any previously scheduled callback is invoked and cleared
before we schedule another one.

Still need to clarify with Matt.

+      api::runtime::RestartOnWatchdog::Results::Create(success, message)));
+}
+
 ExtensionFunction::ResponseAction RuntimeGetPlatformInfoFunction::Run() {
   runtime::PlatformInfo info;
   if (!RuntimeAPI::GetFactoryInstance()