PlzNavigate: update navigation only if it took place.

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 0483c3d0781c3c8393f1c3d29009769c38df48bd..c04ec91a4021c3b4f51844bcaf74ba373cef3441 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -1,3 +1,4 @@
+#include <iostream>

[clamy, 2016/09/23 15:35:38]

Wrong place for include. Please put it just above #include <map> on line 8.

[arthursonzogni, 2016/09/26 08:30:32]

Oops, I didn't have seen I left this debug-line here.

 // Copyright 2013 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.
@@ -3179,23 +3180,24 @@ void RenderFrameImpl::didCreateDataSource(blink::WebLocalFrame* frame,
       ConvertToBlinkTime(navigation_state->common_params().navigation_start));
 
   if (IsBrowserSideNavigationEnabled()) {
-    // Set timing of several events that happened during navigation.
-    // They will be used in blink for the Navigation Timing API.
-    double redirect_start = ConvertToBlinkTime(
-        navigation_state->request_params().navigation_timing.redirect_start);
-    double redirect_end = ConvertToBlinkTime(
-        navigation_state->request_params().navigation_timing.redirect_end);
-    double fetch_start = ConvertToBlinkTime(
-        navigation_state->request_params().navigation_timing.fetch_start);
-    std::vector<GURL> redirectChain =
-        navigation_state->request_params().redirects;
-    redirectChain.push_back(navigation_state->common_params().url);
-
-    datasource->updateNavigation(redirect_start, redirect_end, fetch_start,
-                                 redirectChain);
-    // TODO(clamy) We need to provide additional timing values for the
-    // Navigation Timing API to work with browser-side navigations.
-    // UnloadEventStart and UnloadEventEnd are still missing.
+    const NavigationTiming& timing =
+        navigation_state->request_params().navigation_timing;
+
+    // Inform the datasource of what happened during the browser-side-navigation
+    // only if the navigation actually took place.
+    if (!timing.fetch_start.is_null()) {

[clamy, 2016/09/23 15:35:38]

I would find it a bit more readable if you just did:
// PlzNavigate: if an actual navigation took place, inform the datasource of
what happened in the browser.
if (IsBrowserSideNavigationEnabled() && !timing.fetch_start.is_null()) {
...
}

(Note: I don't see a particular reason to modify what's inside - it makes the
change harder to read).

[arthursonzogni, 2016/09/26 08:30:32]

Done.

+      std::vector<GURL> redirectChain =
+          navigation_state->request_params().redirects;
+      redirectChain.push_back(navigation_state->common_params().url);
+
+      datasource->updateNavigation(ConvertToBlinkTime(timing.redirect_start),
+                                   ConvertToBlinkTime(timing.redirect_end),
+                                   ConvertToBlinkTime(timing.fetch_start),
+                                   redirectChain);
+      // TODO(clamy) We need to provide additional timing values for the
+      // Navigation Timing API to work with browser-side navigations.
+      // UnloadEventStart and UnloadEventEnd are still missing.
+    }
   }
 
   // Create the serviceworker's per-document network observing object if it