Network Service: Make CanonicalCookie times non-nullable.

base::Time already has a null value, so adding mojo's nullable type
support on top of base::Time is more confusing than useful.

Bug: 729800
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I6016a46b48b4efc4e9b584bc5c3062849dab60a0
Reviewed-on: https://chromium-review.googlesource.com/1087539Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564962}

services/network/public/cpp/cookie_manager_mojom_traits.cc

@@ -153,9 +153,9 @@ bool StructTraits<
   if (!cookie.ReadPath(&path))
     return false;
 
-  base::Optional<base::Time> creation_time;
-  base::Optional<base::Time> expiry_time;
-  base::Optional<base::Time> last_access_time;
+  base::Time creation_time;
+  base::Time expiry_time;
+  base::Time last_access_time;
   if (!cookie.ReadCreation(&creation_time))
     return false;
 
@@ -173,10 +173,8 @@ bool StructTraits<
   if (!cookie.ReadPriority(&priority))
     return false;
 
-  *out = net::CanonicalCookie(
-      name, value, domain, path, creation_time ? *creation_time : base::Time(),
-      expiry_time ? *expiry_time : base::Time(),
-      last_access_time ? *last_access_time : base::Time(), cookie.secure(),
+  *out = net::CanonicalCookie(name, value, domain, path, creation_time,
+                              expiry_time, last_access_time, cookie.secure(),
                               cookie.httponly(), site_restrictions, priority);
   return out->IsCanonical();
 }

services/network/public/cpp/cookie_manager_mojom_traits.h

@@ -75,20 +75,14 @@ struct StructTraits<network::mojom::CanonicalCookieDataView,
   static const std::string& path(const net::CanonicalCookie& c) {
     return c.Path();
   }
-  static base::Optional<base::Time> creation(const net::CanonicalCookie& c) {
-    return c.CreationDate().is_null()
-               ? base::Optional<base::Time>()
-               : base::Optional<base::Time>(c.CreationDate());
+  static base::Time creation(const net::CanonicalCookie& c) {
+    return c.CreationDate();
   }
-  static base::Optional<base::Time> expiry(const net::CanonicalCookie& c) {
-    return c.ExpiryDate().is_null()
-               ? base::Optional<base::Time>()
-               : base::Optional<base::Time>(c.ExpiryDate());
+  static base::Time expiry(const net::CanonicalCookie& c) {
+    return c.ExpiryDate();
   }
-  static base::Optional<base::Time> last_access(const net::CanonicalCookie& c) {
-    return c.LastAccessDate().is_null()
-               ? base::Optional<base::Time>()
-               : base::Optional<base::Time>(c.LastAccessDate());
+  static base::Time last_access(const net::CanonicalCookie& c) {
+    return c.LastAccessDate();
   }
   static bool secure(const net::CanonicalCookie& c) { return c.IsSecure(); }
   static bool httponly(const net::CanonicalCookie& c) { return c.IsHttpOnly(); }

services/network/public/mojom/cookie_manager.mojom

@@ -45,9 +45,9 @@ struct CanonicalCookie {
   string value;
   string domain;
   string path;
-  mojo_base.mojom.Time? creation;
-  mojo_base.mojom.Time? expiry;
-  mojo_base.mojom.Time? last_access;
+  mojo_base.mojom.Time creation;
+  mojo_base.mojom.Time expiry;
+  mojo_base.mojom.Time last_access;
   bool secure = false;
   bool httponly = false;
   CookieSameSite site_restrictions = NO_RESTRICTION;

third_party/blink/renderer/modules/cookie_store/cookie_store.cc

@@ -135,10 +135,12 @@ network::mojom::blink::CanonicalCookiePtr ToCanonicalCookie(
       return nullptr;
     }
 
-    if (options.hasExpires())
+    if (options.hasExpires()) {
       canonical_cookie->expiry = WTF::Time::FromJavaTime(options.expires());
-    // The expires option is not set in CookieStoreSetOptions for session
-    // cookies. This is represented by a null expiry field in CanonicalCookie.
+    } else {
+      // Session cookie.
+      canonical_cookie->expiry = WTF::Time();
+    }
   }
 
   if (options.hasDomain()) {
@@ -176,6 +178,10 @@ network::mojom::blink::CanonicalCookiePtr ToCanonicalCookie(
   }
 
   canonical_cookie->httponly = options.httpOnly();
+
+  // The backend does not trust these fields.
+  canonical_cookie->creation = WTF::Time();
+  canonical_cookie->last_access = WTF::Time();
   return canonical_cookie;
 }