predictors: Refactor resource_prefetch_predictor triggering.

chrome/browser/predictors/loading_predictor.cc

 // Copyright 2017 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/predictors/loading_predictor.h"
 
 #include "base/memory/ptr_util.h"
 #include "chrome/browser/predictors/resource_prefetch_predictor.h"
 
 namespace predictors {
 
 LoadingPredictor::LoadingPredictor(const LoadingPredictorConfig& config,
-                                   Profile* profile) {
-  resource_prefetch_predictor_ =
-      base::MakeUnique<ResourcePrefetchPredictor>(config, profile);
-}
+                                   Profile* profile)
+    : resource_prefetch_predictor_(
+          base::MakeUnique<ResourcePrefetchPredictor>(config, profile)) {}

[alexilin, 2017/05/23 11:01:38]

Do you think it's better to construct the pointer through the copy constructor
instead of using the constructor taking a raw pointer?

My suggestion is to use operator new:
resource_prefetch_predictor_(new ResourcePrefetchPredictor(config, profile))

[Benoit L, 2017/05/29 16:45:14]

We are trying to remove "new" as possible from Chrome, see
https://groups.google.com/a/chromium.org/forum/?utm_medium=email&utm_source=f...

In this case the generated code is highly likely to be the same.

 
 LoadingPredictor::~LoadingPredictor() = default;
 
 void LoadingPredictor::PrepareForPageLoad(const GURL& url, HintOrigin origin) {
-  resource_prefetch_predictor_->StartPrefetching(url, origin);
+  if (resource_prefetch_predictor_->CanPrefetchUrlForOrigin(url, origin))

[alexilin, 2017/05/23 11:01:38]

Shouldn't it be like LoadingPredictor tries to get a prediction for Prefetch and
calls ResourcePrefetcher directly if success?
According to our plans, ResourcePrefetchPredictor should eventually disappear
and LoadingPredictor should consult right with GlowplugDatabase.
Do you think we need to keep different types of predictors as abstractions
between LoadingPredictor and Database?

[Benoit L, 2017/05/29 16:45:14]

That's the eventual goal yes, but it doesn't seem that all is ready yet to get
there, hence it stays for now.

+    resource_prefetch_predictor_->StartPrefetching(url, origin);
 }
 
 void LoadingPredictor::CancelPageLoadHint(const GURL& url) {
   resource_prefetch_predictor_->StopPrefetching(url);
 }
 
 void LoadingPredictor::StartInitialization() {
   resource_prefetch_predictor_->StartInitialization();
 }
 
 ResourcePrefetchPredictor* LoadingPredictor::resource_prefetch_predictor()
     const {
   return resource_prefetch_predictor_.get();
 }
 
 void LoadingPredictor::Shutdown() {
   resource_prefetch_predictor_->Shutdown();
 }
 
 }  // namespace predictors