PlzNavigate: update navigation only if it took place.

content/renderer/render_frame_impl.cc

+#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.
 
 #include "content/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 3161 matching lines @@
   }
 
   NavigationStateImpl* navigation_state = static_cast<NavigationStateImpl*>(
       document_state->navigation_state());
 
   // Set the navigation start time in blink.
   datasource->setNavigationStartTime(
       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);
+    const NavigationTiming& timing =
+        navigation_state->request_params().navigation_timing;
 
-    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.
+    // 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
   // does not exist (When navigation happens within a page, the provider already
   // exists).
   if (ServiceWorkerNetworkProvider::FromDocumentState(
           DocumentState::FromDataSource(datasource)))
     return;
 
   ServiceWorkerNetworkProvider::AttachToDocumentState(
@@ skipping 3201 matching lines @@
   // event target. Potentially a Pepper plugin will receive the event.
   // In order to tell whether a plugin gets the last mouse event and which it
   // is, we set |pepper_last_mouse_event_target_| to null here. If a plugin gets
   // the event, it will notify us via DidReceiveMouseEvent() and set itself as
   // |pepper_last_mouse_event_target_|.
   pepper_last_mouse_event_target_ = nullptr;
 #endif
 }
 
 }  // namespace content