predictors: Make speculative_prefetch_predictor work with PlzNavigate

chrome/browser/predictors/resource_prefetch_common.h

 // Copyright 2014 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.
 
 #ifndef CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_COMMON_H_
 #define CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_COMMON_H_
 
 #include <stddef.h>
 
 #include "base/time/time.h"
+#include "components/sessions/core/session_id.h"
 #include "url/gurl.h"
 
 class Profile;
 
 namespace content {
 class WebContents;
 }
 
 namespace predictors {
 
@@ skipping 15 matching lines @@
 enum class PrefetchOrigin { NAVIGATION, EXTERNAL };
 
 // Represents a single navigation for a render frame.
 struct NavigationID {
   NavigationID();
   explicit NavigationID(content::WebContents* web_contents);
   NavigationID(content::WebContents* web_contents,
                const GURL& main_frame_url,
                const base::TimeTicks& creation_time);
   NavigationID(const NavigationID& other);
+
   bool operator<(const NavigationID& rhs) const;
   bool operator==(const NavigationID& rhs) const;
 
-  bool IsSameRenderer(const NavigationID& other) const;
-
   // Returns true iff the render_process_id_, render_frame_id_ and
   // frame_url_ has been set correctly.

[clamy, 2016/12/22 15:52:41]

This comment needs to be updated since render_process_id_ and render_frame_id_
no longer exist.

[ahemery, 2016/12/22 17:12:26]

Done.

   bool is_valid() const;
 
-  int render_process_id;
-  int render_frame_id;
+  SessionID::id_type tab_id;

[clamy, 2016/12/22 15:52:41]

How do you deal with the two concurrent navigation in the main frame case? They
would have been distinguished by the (render_process_id, render_frame_id)
before. IIUC the predictor code correctly, you are only listening for events on
the IO thread by observing the ResourceDispatcherHostDelegate. With PlzNavigate
enabled, it is not possible to know which of two concurrent navigations will
commit only by listening to IO thread events. You need to listen to
WebContentsObserver events on the UI thread.

While we're talking about special cases in navigation, how do you handle
client-side redirects? Are they coalesced with the previous navigation or
treated as a different navigation? IMO there's a rationale for saying they
should be coalesced with the previous navigation.

[Benoit L, 2016/12/22 16:04:10]

Per offline discussion:
- The NavigationID matches not only on the IDs, so this should be fine.
- Client-side redirects are not handled yet, but that's a nice addition for a
future patch, thanks for letting us know.

   GURL main_frame_url;
 
   // NOTE: Even though we store the creation time here, it is not used during
   // comparison of two NavigationIDs because it cannot always be determined
   // correctly.
   base::TimeTicks creation_time;
 };
 
 // Represents the config for the resource prefetch prediction algorithm.
 struct ResourcePrefetchPredictorConfig {
@@ skipping 48 matching lines @@
   // Maximum number of prefetches that can be inflight for a single navigation.
   size_t max_prefetches_inflight_per_navigation;
   // Maximum number of prefetches that can be inflight for a host for a single
   // navigation.
   size_t max_prefetches_inflight_per_host_per_navigation;
 };
 
 }  // namespace predictors
 
 #endif  // CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_COMMON_H_