base: Add CreateTimeValue() and GetValueAsTime()

Currently we only support converting TimeDelta to/from Value, where
2 of the 3 references actually intend to store Time instead of
TimeDelta.

This CL adds functions to help convert Time to/from Value directly.

Also update CreateTimeDeltaValue() and GetValueAsTimeDelta() to use
FromMicroseconds() and InMicroseconds() instead of
FromInternalValue() and ToInternalValue() which are deprecated.

Bug: 917527,928787
Change-Id: Icb0543031ab4f526cfe835d52e85c28efba66863
Reviewed-on: https://chromium-review.googlesource.com/c/1449326Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630014}

base/value_conversions.cc

@@ -45,20 +45,37 @@ bool GetValueAsFilePath(const Value& value, FilePath* file_path) {
   return true;
 }
 
+// It is recommended in time.h to use ToDeltaSinceWindowsEpoch() and
+// FromDeltaSinceWindowsEpoch() for opaque serialization and
+// deserialization of time values.
+Value CreateTimeValue(const Time& time) {
+  return CreateTimeDeltaValue(time.ToDeltaSinceWindowsEpoch());
+}
+
+bool GetValueAsTime(const Value& value, Time* time) {
+  TimeDelta time_delta;
+  if (!GetValueAsTimeDelta(value, &time_delta))
+    return false;
+
+  if (time)
+    *time = Time::FromDeltaSinceWindowsEpoch(time_delta);
+  return true;
+}
+
 // |Value| does not support 64-bit integers, and doubles do not have enough
 // precision, so we store the 64-bit time value as a string instead.
-Value CreateTimeDeltaValue(const TimeDelta& time) {
-  std::string string_value = base::Int64ToString(time.ToInternalValue());
+Value CreateTimeDeltaValue(const TimeDelta& time_delta) {
+  std::string string_value = base::Int64ToString(time_delta.InMicroseconds());
   return Value(string_value);
 }
 
-bool GetValueAsTimeDelta(const Value& value, TimeDelta* time) {
+bool GetValueAsTimeDelta(const Value& value, TimeDelta* time_delta) {
   std::string str;
   int64_t int_value;
   if (!value.GetAsString(&str) || !base::StringToInt64(str, &int_value))
     return false;
-  if (time)
-    *time = TimeDelta::FromInternalValue(int_value);
+  if (time_delta)
+    *time_delta = TimeDelta::FromMicroseconds(int_value);
   return true;
 }
 

base/value_conversions.h

@@ -8,25 +8,43 @@
 // This file contains methods to convert things to a |Value| and back.
 
 #include <memory>
+
 #include "base/base_export.h"
+#include "base/compiler_specific.h"
 
 namespace base {
 
 class FilePath;
+class Time;
 class TimeDelta;
 class UnguessableToken;
 class Value;
 
-// The caller takes ownership of the returned value.
+// In GetValueAs*() functions, the caller owns the object pointed by the output
+// parameter, e.g. |file_path|. If false is returned, the value of the object
+// pointed by the output parameter is not changed. It is okay to pass nullptr to
+// the output parameter.
+
+// Warning: The Values involved could be stored on persistent storage like files
+// on disks. Therefore, changes in implementation could lead to data corruption
+// and must be done with caution.
+
 BASE_EXPORT Value CreateFilePathValue(const FilePath& in_value);
-BASE_EXPORT bool GetValueAsFilePath(const Value& value, FilePath* file_path);
+BASE_EXPORT bool GetValueAsFilePath(const Value& value,
+                                    FilePath* file_path) WARN_UNUSED_RESULT;
+
+BASE_EXPORT Value CreateTimeValue(const Time& time);
+BASE_EXPORT bool GetValueAsTime(const Value& value,
+                                Time* time) WARN_UNUSED_RESULT;
 
-BASE_EXPORT Value CreateTimeDeltaValue(const TimeDelta& time);
-BASE_EXPORT bool GetValueAsTimeDelta(const Value& value, TimeDelta* time);
+BASE_EXPORT Value CreateTimeDeltaValue(const TimeDelta& time_delta);
+BASE_EXPORT bool GetValueAsTimeDelta(const Value& value,
+                                     TimeDelta* time_delta) WARN_UNUSED_RESULT;
 
 BASE_EXPORT Value CreateUnguessableTokenValue(const UnguessableToken& token);
 BASE_EXPORT bool GetValueAsUnguessableToken(const Value& value,
-                                            UnguessableToken* token);
+                                            UnguessableToken* token)
+    WARN_UNUSED_RESULT;
 
 }  // namespace base
 

chrome/browser/devtools/devtools_file_helper.cc

@@ -240,8 +240,11 @@ void DevToolsFileHelper::Save(const std::string& url,
   base::FilePath initial_path;
 
   const base::Value* path_value;
-  if (file_map->Get(base::MD5String(url), &path_value))
-    base::GetValueAsFilePath(*path_value, &initial_path);
+  if (file_map->Get(base::MD5String(url), &path_value)) {
+    // Ignore base::GetValueAsFilePath() failure since we handle empty
+    // |initial_path| below.
+    ignore_result(base::GetValueAsFilePath(*path_value, &initial_path));
+  }
 
   if (initial_path.empty()) {
     GURL gurl(url);

chrome/common/custom_handlers/protocol_handler.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/common/custom_handlers/protocol_handler.h"
 
+#include "base/macros.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/value_conversions.h"
@@ -54,11 +55,9 @@ ProtocolHandler ProtocolHandler::CreateProtocolHandler(
   value->GetString("protocol", &protocol);
   value->GetString("url", &url);
   const base::Value* time_value = value->FindKey("last_modified");
-  if (time_value) {
-    base::TimeDelta time_delta;
-    if (base::GetValueAsTimeDelta(*time_value, &time_delta))
-      time = base::Time::FromDeltaSinceWindowsEpoch(time_delta);
-  }
+  // Treat invalid times as the default value.
+  if (time_value)
+    ignore_result(base::GetValueAsTime(*time_value, &time));
   return ProtocolHandler(protocol, GURL(url), time);
 }
 
@@ -74,8 +73,7 @@ std::unique_ptr<base::DictionaryValue> ProtocolHandler::Encode() const {
   auto d = std::make_unique<base::DictionaryValue>();
   d->SetString("protocol", protocol_);
   d->SetString("url", url_.spec());
-  d->SetKey("last_modified", base::CreateTimeDeltaValue(
-                                 last_modified_.ToDeltaSinceWindowsEpoch()));
+  d->SetKey("last_modified", base::CreateTimeValue(last_modified_));
   return d;
 }
 

components/translate/core/browser/translate_prefs.cc

@@ -11,6 +11,7 @@
 
 #include "base/feature_list.h"
 #include "base/i18n/rtl.h"
+#include "base/macros.h"
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_piece.h"
@@ -75,18 +76,6 @@ const char TranslatePrefs::kPrefExplicitLanguageAskShown[] =
 // * translate_too_often_denied
 // * translate_language_blacklist
 
-namespace {
-
-// Extract a timestamp from a base::Value.
-// Will return base::Time() if no valid timestamp exists.
-base::Time GetTimeStamp(const base::Value& value) {
-  base::TimeDelta delta;
-  base::GetValueAsTimeDelta(value, &delta);
-  return base::Time::FromDeltaSinceWindowsEpoch(delta);
-}
-
-}  // namespace
-
 const base::Feature kRegionalLocalesAsDisplayUI{
     "RegionalLocalesAsDisplayUI", base::FEATURE_ENABLED_BY_DEFAULT};
 
@@ -484,8 +473,7 @@ void TranslatePrefs::BlacklistSite(const std::string& site) {
   BlacklistValue(kPrefTranslateSiteBlacklistDeprecated, site);
   DictionaryPrefUpdate update(prefs_, kPrefTranslateSiteBlacklistWithTime);
   base::DictionaryValue* dict = update.Get();
-  dict->SetKey(site, base::CreateTimeDeltaValue(
-                         base::Time::Now().ToDeltaSinceWindowsEpoch()));
+  dict->SetKey(site, base::CreateTimeValue(base::Time::Now()));
 }
 
 void TranslatePrefs::RemoveSiteFromBlacklist(const std::string& site) {
@@ -503,7 +491,9 @@ std::vector<std::string> TranslatePrefs::GetBlacklistedSitesBetween(
   auto* dict = prefs_->GetDictionary(kPrefTranslateSiteBlacklistWithTime);
   for (const auto& entry : *dict) {
     std::string site = entry.first;
-    base::Time time = GetTimeStamp(*entry.second);
+    base::Time time;
+    // TODO(crbug.com/928787): Handle base::GetValueAsTime() failure.
+    ignore_result(base::GetValueAsTime(*entry.second, &time));
     if (begin <= time && time < end)
       result.push_back(site);
   }

extensions/browser/api/alarms/alarm_manager.cc

@@ -11,6 +11,7 @@
 #include "base/bind.h"
 #include "base/json/json_writer.h"
 #include "base/lazy_instance.h"
+#include "base/macros.h"
 #include "base/time/clock.h"
 #include "base/time/default_clock.h"
 #include "base/time/time.h"
@@ -76,8 +77,11 @@ AlarmManager::AlarmList AlarmsFromValue(const std::string extension_id,
     if (list->GetDictionary(i, &alarm_dict) &&
         alarms::Alarm::Populate(*alarm_dict, alarm->js_alarm.get())) {
       const base::Value* time_value = nullptr;
-      if (alarm_dict->Get(kAlarmGranularity, &time_value))
-        base::GetValueAsTimeDelta(*time_value, &alarm->granularity);
+      if (alarm_dict->Get(kAlarmGranularity, &time_value)) {
+        // It's okay to ignore the failure since we have minimum granularity.
+        ignore_result(
+            base::GetValueAsTimeDelta(*time_value, &alarm->granularity));
+      }
       alarm->minimum_granularity = base::TimeDelta::FromSecondsD(
           (is_unpacked ? alarms_api_constants::kDevDelayMinimum
                        : alarms_api_constants::kReleaseDelayMinimum) *