Add WARN_UNUSED_RESULT to base::Time methods that return bool.

base::Time::FromString() and base::Time::FromUTCString() might fail
at runtime on bad input. This adds a WARN_UNUSED_RESULT to their
declarations to ensure that all callers properly handle the result.

Apart from unit-tests, this changes two things:

- DataUseTracker::RemoveExpiredEntriesForPref() will also
  remove any entries with an invalid date string.

  This doesn't change the runtime behaviour though, because
  a failure in base::Time::FromUTCString() would keep the
  local |key_date| variable to 0, which would have been
  considered too old anyway.

- VariationsSeedStore::ImportFirstRunJavaSeed() has
  a new failure mode triggered by an invalid response
  date |FIRST_RUN_SEED_IMPORT_FAIL_INVALID_RESPONSE_DATE|

  It looks like the FirstRunResult enum values are only
  used on Android to report UMA metrics, so ensure that
  the new constant is added at the end of the list to
  avoid generating confusing histograms.

- PexeDownloader::didReceiveResponse() still ignores
  invalid HTTP response 'last-modified' header dates.
  As with RemoveExpiredEntriesForPref() this doesn't
  change the runtime behaviour, but it is hard to see
  whether this is desirable or dangerous.

- ConvertRequestValueToFileInfo() still ignores
  invalid date strings, as mentioned by the comment
  above the base::Time::FromString() line.

BUG=669625

Review-Url: https://codereview.chromium.org/2605293002
Cr-Commit-Position: refs/heads/master@{#443353}

base/time/time.h

@@ -557,10 +557,12 @@ class BASE_EXPORT Time : public time_internal::TimeBase<Time> {
   // specified in RFC822) is treated as if the timezone is not specified.
   // TODO(iyengar) Move the FromString/FromTimeT/ToTimeT/FromFileTime to
   // a new time converter class.
-  static bool FromString(const char* time_string, Time* parsed_time) {
+  static bool FromString(const char* time_string,
+                         Time* parsed_time) WARN_UNUSED_RESULT {
     return FromStringInternal(time_string, true, parsed_time);
   }
-  static bool FromUTCString(const char* time_string, Time* parsed_time) {
+  static bool FromUTCString(const char* time_string,
+                            Time* parsed_time) WARN_UNUSED_RESULT {
     return FromStringInternal(time_string, false, parsed_time);
   }
 
@@ -603,10 +605,11 @@ class BASE_EXPORT Time : public time_internal::TimeBase<Time> {
   // timezone is not specified.
   static bool FromStringInternal(const char* time_string,
                                  bool is_local,
-                                 Time* parsed_time);
+                                 Time* parsed_time) WARN_UNUSED_RESULT;
 
   // Comparison does not consider |day_of_week| when doing the operation.
-  static bool ExplodedMostlyEquals(const Exploded& lhs, const Exploded& rhs);
+  static bool ExplodedMostlyEquals(const Exploded& lhs,
+                                   const Exploded& rhs) WARN_UNUSED_RESULT;
 };
 
 // static
@@ -713,14 +716,14 @@ class BASE_EXPORT TimeTicks : public time_internal::TimeBase<TimeTicks> {
   // Now() will return high resolution values. Note that, on systems where the
   // high resolution clock works but is deemed inefficient, the low resolution
   // clock will be used instead.
-  static bool IsHighResolution();
+  static bool IsHighResolution() WARN_UNUSED_RESULT;
 
   // Returns true if TimeTicks is consistent across processes, meaning that
   // timestamps taken on different processes can be safely compared with one
   // another. (Note that, even on platforms where this returns true, time values
   // from different threads that are within one tick of each other must be
   // considered to have an ambiguous ordering.)
-  static bool IsConsistentAcrossProcesses();
+  static bool IsConsistentAcrossProcesses() WARN_UNUSED_RESULT;
 
 #if defined(OS_WIN)
   // Translates an absolute QPC timestamp into a TimeTicks value. The returned
@@ -781,7 +784,7 @@ class BASE_EXPORT ThreadTicks : public time_internal::TimeBase<ThreadTicks> {
   }
 
   // Returns true if ThreadTicks::Now() is supported on this system.
-  static bool IsSupported() {
+  static bool IsSupported() WARN_UNUSED_RESULT {
 #if (defined(_POSIX_THREAD_CPUTIME) && (_POSIX_THREAD_CPUTIME >= 0)) || \
     (defined(OS_MACOSX) && !defined(OS_IOS)) || defined(OS_ANDROID)
     return true;
@@ -832,7 +835,7 @@ class BASE_EXPORT ThreadTicks : public time_internal::TimeBase<ThreadTicks> {
   // allow testing.
   static double TSCTicksPerSecond();
 
-  static bool IsSupportedWin();
+  static bool IsSupportedWin() WARN_UNUSED_RESULT;
   static void WaitUntilInitializedWin();
 #endif
 };

chrome/browser/chromeos/file_system_provider/operations/get_metadata.cc

@@ -10,6 +10,7 @@
 #include <string>
 #include <utility>
 
+#include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "chrome/common/extensions/api/file_system_provider.h"
 #include "chrome/common/extensions/api/file_system_provider_internal.h"
@@ -53,8 +54,8 @@ bool ConvertRequestValueToFileInfo(std::unique_ptr<RequestValue> value,
     // Allow to pass invalid modification time, since there is no way to verify
     // it easily on any earlier stage.
     base::Time output_modification_time;
-    base::Time::FromString(input_modification_time.c_str(),
-                           &output_modification_time);
+    ignore_result(base::Time::FromString(input_modification_time.c_str(),
+                                         &output_modification_time));
     output->modification_time.reset(new base::Time(output_modification_time));
   }
 

chrome/browser/chromeos/mobile_config_unittest.cc

@@ -147,7 +147,7 @@ TEST(MobileConfigTest, Basic) {
             deal->GetLocalizedString("en", "notification_text"));
 
   base::Time reference_time;
-  base::Time::FromString("31/12/2099 0:00", &reference_time);
+  EXPECT_TRUE(base::Time::FromString("31/12/2099 0:00", &reference_time));
   EXPECT_EQ(reference_time, deal->expire_date());
 
   const MobileConfig::LocaleConfig* locale_config;
@@ -232,7 +232,7 @@ TEST(MobileConfigTest, LocalConfig) {
   EXPECT_EQ("default_text from local.",
             deal->GetLocalizedString("en", "notification_text"));
   base::Time reference_time;
-  base::Time::FromString("31/12/2099 0:00", &reference_time);
+  EXPECT_TRUE(base::Time::FromString("31/12/2099 0:00", &reference_time));
   EXPECT_EQ(reference_time, deal->expire_date());
 
   // Now reload same global/local config files but with proper initial locale.

components/data_reduction_proxy/core/browser/data_reduction_proxy_compression_stats_unittest.cc

@@ -114,7 +114,7 @@ const char kLastUpdateTime[] = "Wed, 18 Sep 2013 03:45:26";
 class DataReductionProxyCompressionStatsTest : public testing::Test {
  protected:
   DataReductionProxyCompressionStatsTest() {
-    base::Time::FromString(kLastUpdateTime, &now_);
+    EXPECT_TRUE(base::Time::FromString(kLastUpdateTime, &now_));
   }
 
   void SetUp() override {

components/metrics/data_use_tracker.cc

@@ -118,8 +118,8 @@ void DataUseTracker::RemoveExpiredEntriesForPref(const std::string& pref_name) {
   for (base::DictionaryValue::Iterator it(*user_pref_dict); !it.IsAtEnd();
        it.Advance()) {
     base::Time key_date;
-    base::Time::FromUTCString(it.key().c_str(), &key_date);
-    if (key_date > week_ago)
+    if (base::Time::FromUTCString(it.key().c_str(), &key_date) &&
+        key_date > week_ago)
       user_pref_new_dict.Set(it.key(), it.value().CreateDeepCopy());
   }
   local_state_->Set(pref_name, user_pref_new_dict);

components/metrics/data_use_tracker_unittest.cc

@@ -51,7 +51,7 @@ class FakeDataUseTracker : public DataUseTracker {
 
   base::Time GetCurrentMeasurementDate() const override {
     base::Time today_for_test;
-    base::Time::FromUTCString(kTodayStr, &today_for_test);
+    EXPECT_TRUE(base::Time::FromUTCString(kTodayStr, &today_for_test));
     return today_for_test;
   }
 

components/nacl/browser/pnacl_translation_cache_unittest.cc

@@ -135,7 +135,7 @@ TEST(PnaclTranslationCacheKeyTest, CacheKeyTest) {
   info.opt_level = 0;
   info.sandbox_isa = "x86-32";
   std::string test_time("Wed, 15 Nov 1995 06:25:24 GMT");
-  base::Time::FromString(test_time.c_str(), &info.last_modified);
+  EXPECT_TRUE(base::Time::FromString(test_time.c_str(), &info.last_modified));
   // Basic check for URL and time components
   EXPECT_EQ("ABI:0;opt:0;URL:http://www.google.com/;"
             "modified:1995:11:15:6:25:24:0:UTC;etag:;"
@@ -204,7 +204,7 @@ TEST(PnaclTranslationCacheKeyTest, CacheKeyTest) {
             "sandbox:x86-32;extra_flags:-mavx-neon;",
             PnaclTranslationCache::GetKey(info));
   test_time.assign("Fri, 29 Feb 2008 13:04:12 GMT");
-  base::Time::FromString(test_time.c_str(), &info.last_modified);
+  EXPECT_TRUE(base::Time::FromString(test_time.c_str(), &info.last_modified));
   EXPECT_EQ("ABI:2;opt:2;URL:http://www.google.com/;"
             "modified:2008:2:29:13:4:12:0:UTC;etag:etag;"
             "sandbox:x86-32;extra_flags:-mavx-neon;",

components/nacl/renderer/ppb_nacl_private_impl.cc

@@ -1620,10 +1620,16 @@ class PexeDownloader : public blink::WebAssociatedURLLoaderClient {
     url_loader_->setDefersLoading(true);
 
     std::string etag = response.httpHeaderField("etag").utf8();
+
+    // Parse the "last-modified" date string. An invalid string will result
+    // in a base::Time value of 0, which is supported by the only user of
+    // the |CacheInfo::last_modified| field (see
+    // pnacl::PnaclTranslationCache::GetKey()).
     std::string last_modified =
         response.httpHeaderField("last-modified").utf8();
     base::Time last_modified_time;
-    base::Time::FromString(last_modified.c_str(), &last_modified_time);
+    ignore_result(
+        base::Time::FromString(last_modified.c_str(), &last_modified_time));
 
     bool has_no_store_header = false;
     std::string cache_control =

components/variations/variations_seed_store.cc

@@ -139,6 +139,7 @@ enum FirstRunResult {
   FIRST_RUN_SEED_IMPORT_FAIL_NO_CALLBACK,
   FIRST_RUN_SEED_IMPORT_FAIL_NO_FIRST_RUN_SEED,
   FIRST_RUN_SEED_IMPORT_FAIL_STORE_FAILED,
+  FIRST_RUN_SEED_IMPORT_FAIL_INVALID_RESPONSE_DATE,
   FIRST_RUN_RESULT_ENUM_SIZE,
 };
 
@@ -367,7 +368,11 @@ void VariationsSeedStore::ImportFirstRunJavaSeed() {
   }
 
   base::Time current_date;
-  base::Time::FromUTCString(response_date.c_str(), &current_date);
+  if (!base::Time::FromUTCString(response_date.c_str(), &current_date)) {
+    RecordFirstRunResult(FIRST_RUN_SEED_IMPORT_FAIL_INVALID_RESPONSE_DATE);
+    LOG(WARNING) << "Invalid response date: " << response_date;
+    return;
+  }
 
   if (!StoreSeedData(seed_data, seed_signature, seed_country, current_date,
                      false, is_gzip_compressed, nullptr)) {

content/browser/blob_storage/blob_storage_context_unittest.cc

@@ -591,8 +591,8 @@ TEST_F(BlobStorageContextTest, CompoundBlobs) {
 
   // Setup a set of blob data for testing.
   base::Time time1, time2;
-  base::Time::FromString("Tue, 15 Nov 1994, 12:45:26 GMT", &time1);
-  base::Time::FromString("Mon, 14 Nov 1994, 11:30:49 GMT", &time2);
+  ASSERT_TRUE(base::Time::FromString("Tue, 15 Nov 1994, 12:45:26 GMT", &time1));
+  ASSERT_TRUE(base::Time::FromString("Mon, 14 Nov 1994, 11:30:49 GMT", &time2));
 
   BlobDataBuilder blob_data1(kId1);
   blob_data1.AppendData("Data1");

content/browser/loader/upload_data_stream_builder_unittest.cc

@@ -104,7 +104,8 @@ TEST(UploadDataStreamBuilderTest,
 
     const uint64_t kZeroLength = 0;
     base::Time blob_time;
-    base::Time::FromString("Tue, 15 Nov 1994, 12:45:26 GMT", &blob_time);
+    ASSERT_TRUE(
+        base::Time::FromString("Tue, 15 Nov 1994, 12:45:26 GMT", &blob_time));
     ASSERT_TRUE(base::TouchFile(test_blob_path, blob_time, blob_time));
 
     BlobStorageContext blob_storage_context;

net/http/http_response_headers_unittest.cc

@@ -780,9 +780,12 @@ TEST_P(RequiresValidationTest, RequiresValidation) {
   const RequiresValidationTestData test = GetParam();
 
   base::Time request_time, response_time, current_time;
-  base::Time::FromString("Wed, 28 Nov 2007 00:40:09 GMT", &request_time);
-  base::Time::FromString("Wed, 28 Nov 2007 00:40:12 GMT", &response_time);
-  base::Time::FromString("Wed, 28 Nov 2007 00:45:20 GMT", &current_time);
+  ASSERT_TRUE(
+      base::Time::FromString("Wed, 28 Nov 2007 00:40:09 GMT", &request_time));
+  ASSERT_TRUE(
+      base::Time::FromString("Wed, 28 Nov 2007 00:40:12 GMT", &response_time));
+  ASSERT_TRUE(
+      base::Time::FromString("Wed, 28 Nov 2007 00:45:20 GMT", &current_time));
 
   std::string headers(test.headers);
   HeadersToRaw(&headers);