Update PageLoadTiming to use base::Optional

components/page_load_metrics/browser/metrics_web_contents_observer.cc

 // Copyright 2015 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 "components/page_load_metrics/browser/metrics_web_contents_observer.h"
 
 #include <algorithm>
+#include <ostream>
 #include <string>
 #include <utility>
 
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/user_metrics.h"
 #include "components/page_load_metrics/browser/page_load_metrics_util.h"
 #include "components/page_load_metrics/common/page_load_metrics_messages.h"
@@ skipping 28 matching lines @@
     "PageLoad.Internal.ProvisionalAbortChainSize.SameURL";
 const char kAbortChainSizeNoCommit[] =
     "PageLoad.Internal.ProvisionalAbortChainSize.NoCommit";
 const char kClientRedirectDelayAfterPaint[] =
     "PageLoad.Internal.ClientRedirectDelayAfterPaint";
 
 }  // namespace internal
 
 namespace {
 
+// Helper to allow use of Optional<> values in LOG() messages.
+std::ostream& operator<<(std::ostream& os,
+                         const base::Optional<base::TimeDelta>& opt) {
+  if (opt)
+    os << opt.value();
+  else
+    os << "(unset)";
+  return os;
+}
+

[shivanisha, 2016/07/07 15:38:57]

Would the operator definition be a good fit for base/optional.h?

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

I had wanted to do this as well. I asked the base/OWNERS about this - they said
they only want to include things in base/optional.h that are part of
std::optional, so we can port from base::Optional to std::optional when it's
available in the Chromium build. They suggested doing this for now. There's a
bit more discussion on this in https://codereview.chromium.org/2109063006 (which
I closed without committing, based on feedback from base owners).

 // The url we see from the renderer side is not always the same as what
 // we see from the browser side (e.g. chrome://newtab). We want to be
 // sure here that we aren't logging UMA for internal pages.
 bool IsRelevantNavigation(content::NavigationHandle* navigation_handle,
                           const GURL& browser_url,
                           const std::string& mime_type) {
   DCHECK(navigation_handle->HasCommitted());
   return navigation_handle->IsInMainFrame() &&
          !navigation_handle->IsSamePage() &&
          !navigation_handle->IsErrorPage() &&
          navigation_handle->GetURL().SchemeIsHTTPOrHTTPS() &&
          browser_url.SchemeIsHTTPOrHTTPS() &&
          (mime_type == "text/html" || mime_type == "application/xhtml+xml");
 }
 
 // If second is non-zero, first must also be non-zero and less than or equal to
 // second.
-bool EventsInOrder(base::TimeDelta first, base::TimeDelta second) {
-  if (second.is_zero()) {
+bool EventsInOrder(const base::Optional<base::TimeDelta>& first,
+                   const base::Optional<base::TimeDelta>& second) {
+  if (!second) {
     return true;
   }
-  return !first.is_zero() && first <= second;
+  return first && first <= second;
 }
 
 bool IsValidPageLoadTiming(const PageLoadTiming& timing) {
   if (timing.IsEmpty())
     return false;
 
   // If we have a non-empty timing, it should always have a navigation start.
   if (timing.navigation_start.is_null()) {
     NOTREACHED() << "Received null navigation_start.";
     return false;
@@ skipping 16 matching lines @@
                 << " for parse_start " << timing.parse_start;
     return false;
   }
 
   if (!EventsInOrder(timing.parse_start, timing.parse_stop)) {
     NOTREACHED() << "Invalid parse_start " << timing.parse_start
                  << " for parse_stop " << timing.parse_stop;
     return false;
   }
 
-  if (!timing.parse_stop.is_zero()) {
+  if (timing.parse_stop) {
     const base::TimeDelta parse_duration =
-        timing.parse_stop - timing.parse_start;
+        timing.parse_stop.value() - timing.parse_start.value();
     if (timing.parse_blocked_on_script_load_duration > parse_duration) {
       NOTREACHED() << "Invalid parse_blocked_on_script_load_duration "
                    << timing.parse_blocked_on_script_load_duration
                    << " for parse duration " << parse_duration;
       return false;
     }
   }
 
   if (timing.parse_blocked_on_script_load_from_document_write_duration >
       timing.parse_blocked_on_script_load_duration) {
@@ skipping 78 matching lines @@
                          aborted_chain_size_same_url);
   }
 }
 
 void DispatchObserverTimingCallbacks(PageLoadMetricsObserver* observer,
                                      const PageLoadTiming& last_timing,
                                      const PageLoadTiming& new_timing,
                                      const PageLoadMetadata& last_metadata,
                                      const PageLoadExtraInfo& extra_info) {
   observer->OnTimingUpdate(new_timing, extra_info);
-  if (!new_timing.dom_content_loaded_event_start.is_zero() &&
-      last_timing.dom_content_loaded_event_start.is_zero())
+  if (new_timing.dom_content_loaded_event_start &&
+      !last_timing.dom_content_loaded_event_start)
     observer->OnDomContentLoadedEventStart(new_timing, extra_info);
-  if (!new_timing.load_event_start.is_zero() &&
-      last_timing.load_event_start.is_zero())
+  if (new_timing.load_event_start && !last_timing.load_event_start)
     observer->OnLoadEventStart(new_timing, extra_info);
-  if (!new_timing.first_layout.is_zero() && last_timing.first_layout.is_zero())
+  if (new_timing.first_layout && !last_timing.first_layout)
     observer->OnFirstLayout(new_timing, extra_info);
-  if (!new_timing.first_paint.is_zero() && last_timing.first_paint.is_zero())
+  if (new_timing.first_paint && !last_timing.first_paint)
     observer->OnFirstPaint(new_timing, extra_info);
-  if (!new_timing.first_text_paint.is_zero() &&
-      last_timing.first_text_paint.is_zero())
+  if (new_timing.first_text_paint && !last_timing.first_text_paint)
     observer->OnFirstTextPaint(new_timing, extra_info);
-  if (!new_timing.first_image_paint.is_zero() &&
-      last_timing.first_image_paint.is_zero())
+  if (new_timing.first_image_paint && !last_timing.first_image_paint)
     observer->OnFirstImagePaint(new_timing, extra_info);
-  if (!new_timing.first_contentful_paint.is_zero() &&
-      last_timing.first_contentful_paint.is_zero())
+  if (new_timing.first_contentful_paint && !last_timing.first_contentful_paint)
     observer->OnFirstContentfulPaint(new_timing, extra_info);
-  if (!new_timing.parse_start.is_zero() && last_timing.parse_start.is_zero())
+  if (new_timing.parse_start && !last_timing.parse_start)
     observer->OnParseStart(new_timing, extra_info);
-  if (!new_timing.parse_stop.is_zero() && last_timing.parse_stop.is_zero())
+  if (new_timing.parse_stop && !last_timing.parse_stop)
     observer->OnParseStop(new_timing, extra_info);
   if (extra_info.metadata.behavior_flags != last_metadata.behavior_flags)
     observer->OnLoadingBehaviorObserved(extra_info);
 }
 
 }  // namespace
 
 PageLoadTracker::PageLoadTracker(
     bool in_foreground,
     PageLoadMetricsEmbedderInterface* embedder_interface,
@@ skipping 136 matching lines @@
 
 void PageLoadTracker::OnInputEvent(const blink::WebInputEvent& event) {
   for (const auto& observer : observers_) {
     observer->OnUserInput(event);
   }
 }
 
 void PageLoadTracker::NotifyClientRedirectTo(
     const PageLoadTracker& destination) {
   base::TimeDelta redirect_delay_after_paint;
-  if (!timing_.first_paint.is_zero()) {
-    base::TimeTicks first_paint_time = navigation_start() + timing_.first_paint;
+  if (timing_.first_paint) {
+    base::TimeTicks first_paint_time =
+        navigation_start() + timing_.first_paint.value();
     if (destination.navigation_start() > first_paint_time)
       redirect_delay_after_paint =
           destination.navigation_start() - first_paint_time;
   }
   PAGE_LOAD_HISTOGRAM(internal::kClientRedirectDelayAfterPaint,
                       redirect_delay_after_paint);
 }
 
 bool PageLoadTracker::UpdateTiming(const PageLoadTiming& new_timing,
                                    const PageLoadMetadata& new_metadata) {
@@ skipping 507 matching lines @@
 
   if (!committed_load_->UpdateTiming(timing, metadata)) {
     // If the page load tracker cannot update its timing, something is wrong
     // with the IPC (it's from another load, or it's invalid in some other way).
     // We expect this to be a rare occurrence.
     RecordInternalError(ERR_BAD_TIMING_IPC);
   }
 }
 
 }  // namespace page_load_metrics