Update PageLoadTiming to use base::Optional

chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.h"
 
 #include <string>
 
 #include "base/time/time.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h"
@@ skipping 10 matching lines @@
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/navigation_data.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
 
 namespace data_reduction_proxy {
 
 namespace {
 
 bool ShouldRecordHistogram(const DataReductionProxyData* data,
-                           const base::TimeDelta& event,
+                           const base::Optional<base::TimeDelta>& event,
                            const page_load_metrics::PageLoadExtraInfo& info) {
   return data && data->used_data_reduction_proxy() &&
-         WasStartedInForegroundEventInForeground(event, info);
+         WasStartedInForegroundOptionalEventInForeground(event, info);
 }
 
 // A macro is needed because PAGE_LOAD_HISTOGRAM creates a static instance of
 // the histogram. A distinct histogram is needed for each place that calls
 // RECORD_HISTOGRAMS_FOR_SUFFIX.
 #define RECORD_HISTOGRAMS_FOR_SUFFIX(data, event, info, histogram_suffix)   \
   do {                                                                      \
     if (ShouldRecordHistogram(data.get(), event, info)) {                   \
       PAGE_LOAD_HISTOGRAM(                                                  \
           std::string(internal::kHistogramDataReductionProxyPrefix)         \
               .append(histogram_suffix),                                    \
-          event);                                                           \
+          event.value());                                                   \
       if (data->lofi_requested()) {                                         \
         PAGE_LOAD_HISTOGRAM(                                                \
             std::string(internal::kHistogramDataReductionProxyLoFiOnPrefix) \
                 .append(histogram_suffix),                                  \
-            event);                                                         \
+            event.value());                                                 \
       }                                                                     \
     }                                                                       \
   } while (false)
 
 }  // namespace
 
 namespace internal {
 
 const char kHistogramDataReductionProxyPrefix[] =
     "PageLoad.Clients.DataReductionProxy.";
@@ skipping 61 matching lines @@
   if (data_reduction_proxy::params::IsIncludedInHoldbackFieldTrial() ||
       data_reduction_proxy::params::IsIncludedInTamperDetectionExperiment()) {
     return;
   }
   // Only consider timing events that happened before the first background
   // event.
   base::TimeDelta response_start;
   base::TimeDelta load_event_start;
   base::TimeDelta first_image_paint;
   base::TimeDelta first_contentful_paint;
-  if (WasStartedInForegroundEventInForeground(timing.response_start, info))
-    response_start = timing.response_start;
-  if (WasStartedInForegroundEventInForeground(timing.load_event_start, info))
-    load_event_start = timing.load_event_start;
-  if (WasStartedInForegroundEventInForeground(timing.first_image_paint, info))
-    first_image_paint = timing.first_image_paint;
-  if (WasStartedInForegroundEventInForeground(timing.first_contentful_paint,
-                                              info)) {
-    first_contentful_paint = timing.first_contentful_paint;
+  if (WasStartedInForegroundOptionalEventInForeground(timing.response_start,
+                                                      info))
+    response_start = timing.response_start.value();

[RyanSturm, 2016/07/06 17:10:51]

nit: add braces for these since the if is spanning 2 lines now.

[Bryan McQuade, 2016/07/08 18:39:35]

Done (and for other cases below)

+  if (WasStartedInForegroundOptionalEventInForeground(timing.load_event_start,
+                                                      info))
+    load_event_start = timing.load_event_start.value();
+  if (WasStartedInForegroundOptionalEventInForeground(timing.first_image_paint,
+                                                      info))
+    first_image_paint = timing.first_image_paint.value();
+  if (WasStartedInForegroundOptionalEventInForeground(
+          timing.first_contentful_paint, info)) {
+    first_contentful_paint = timing.first_contentful_paint.value();
   }
+  // TODO(ryansturm): consider changing DataReductionProxyPageLoadTiming to take

[RyanSturm, 2016/07/06 17:10:51]

I opened a bug, can you add crbug.com/626040 to the comment

[Bryan McQuade, 2016/07/08 18:39:35]

Thanks! Comment updated.

+  // base::Optionals, so we can distinguish zeroes from unset timings.
   DataReductionProxyPageLoadTiming data_reduction_proxy_timing(
       timing.navigation_start, response_start, load_event_start,
       first_image_paint, first_contentful_paint);
   GetPingbackClient()->SendPingback(*data_, data_reduction_proxy_timing);
 }
 
 void DataReductionProxyMetricsObserver::OnDomContentLoadedEventStart(
     const page_load_metrics::PageLoadTiming& timing,
     const page_load_metrics::PageLoadExtraInfo& info) {
   RECORD_HISTOGRAMS_FOR_SUFFIX(
@@ skipping 52 matching lines @@
 
 DataReductionProxyPingbackClient*
 DataReductionProxyMetricsObserver::GetPingbackClient() const {
   return DataReductionProxyChromeSettingsFactory::GetForBrowserContext(
              browser_context_)
       ->data_reduction_proxy_service()
       ->pingback_client();
 }
 
 }  // namespace data_reduction_proxy