predictors: Take the priority into account for scoring resources.

chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc

 // 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.
 
 #include <memory>
 #include <set>
 #include <utility>
 #include <vector>
 
 #include "base/message_loop/message_loop.h"
@@ skipping 557 matching lines @@
 
 void ResourcePrefetchPredictorTablesTest::ReopenDatabase() {
   db_.reset(new PredictorDatabase(&profile_));
   base::RunLoop().RunUntilIdle();
   tables_ = db_->resource_prefetch_tables();
 }
 
 // Test cases.
 
 TEST_F(ResourcePrefetchPredictorTablesTest, ComputeResourceScore) {
-  ResourceData js_resource = CreateResourceData(
-      "http://www.resources.google.com/script.js",
-      content::RESOURCE_TYPE_SCRIPT, 11, 0, 0, 1., net::MEDIUM, false, false);
-  ResourceData image_resource = CreateResourceData(
-      "http://www.resources.google.com/image.jpg", content::RESOURCE_TYPE_IMAGE,
-      11, 0, 0, 1., net::MEDIUM, false, false);
-  ResourceData css_resource =
-      CreateResourceData("http://www.resources.google.com/stylesheet.css",
-                         content::RESOURCE_TYPE_STYLESHEET, 11, 0, 0, 1.,
-                         net::MEDIUM, false, false);
-  ResourceData font_resource =
-      CreateResourceData("http://www.resources.google.com/font.woff",
-                         content::RESOURCE_TYPE_FONT_RESOURCE, 11, 0, 0, 1.,
-                         net::MEDIUM, false, false);
-  float js_resource_score =
-      ResourcePrefetchPredictorTables::ComputeResourceScore(js_resource);
-  float css_resource_score =
-      ResourcePrefetchPredictorTables::ComputeResourceScore(css_resource);
-  float font_resource_score =
-      ResourcePrefetchPredictorTables::ComputeResourceScore(font_resource);
-  float image_resource_score =
-      ResourcePrefetchPredictorTables::ComputeResourceScore(image_resource);
+  auto compute_score = [](net::RequestPriority priority,

[pasko, 2016/10/03 16:48:20]

Why would lamda be a necessary complication here?

compute_score(params) is more surprising to read than ComputeScore(params).
Let's make it a regular function with naming in function style.

[Benoit L, 2016/10/04 09:49:54]

It was to make the function extra-duper local.
Also, ResourcePrefetchPredictoTables::ComputeResourceScore() is private, so this
is actually less verbose. Finally, it puts the definition right next to its use.

wdyt?

[pasko, 2016/10/04 11:10:36]

Thank you for explanation.

For my personal taste: lambda syntax is annoying, rarely increases readability
when it is not below maybe-5-liner.

For consistency with other code: an extra ForTesting method is usually used in
this case.

Style guide: allows this type of use.

Current use in Chromium:

$ git grep 'auto .* = \[.*\]' | wc -l
111

Agreed that such lambda is better than alternatives in this case, thanks again!

+                          content::ResourceType resource_type,
+                          double average_position) {
+    return ResourcePrefetchPredictorTables::ComputeResourceScore(
+        CreateResourceData("", resource_type, 0, 0, 0, average_position,
+                           priority, false, false));
+  };
 
-  EXPECT_TRUE(js_resource_score == css_resource_score);
-  EXPECT_TRUE(js_resource_score == font_resource_score);
-  EXPECT_NEAR(199., js_resource_score, 1e-4);
-  EXPECT_NEAR(99., image_resource_score, 1e-4);
+  // Priority is more important than the rest.
+  EXPECT_TRUE(compute_score(net::HIGHEST, content::RESOURCE_TYPE_SCRIPT, 1.) >
+              compute_score(net::HIGHEST, content::RESOURCE_TYPE_IMAGE, 42.));
+
+  EXPECT_TRUE(compute_score(net::HIGHEST, content::RESOURCE_TYPE_IMAGE, 42.) >
+              compute_score(net::MEDIUM, content::RESOURCE_TYPE_SCRIPT, 1.));
+  EXPECT_TRUE(compute_score(net::HIGHEST, content::RESOURCE_TYPE_IMAGE, 42.) >
+              compute_score(net::LOW, content::RESOURCE_TYPE_SCRIPT, 1.));
+  EXPECT_TRUE(compute_score(net::HIGHEST, content::RESOURCE_TYPE_IMAGE, 42.) >
+              compute_score(net::LOWEST, content::RESOURCE_TYPE_SCRIPT, 1.));
+  EXPECT_TRUE(compute_score(net::HIGHEST, content::RESOURCE_TYPE_IMAGE, 42.) >
+              compute_score(net::IDLE, content::RESOURCE_TYPE_SCRIPT, 1.));
+
+  // Scripts and stylesheets are equivalent.
+  EXPECT_NEAR(
+      compute_score(net::HIGHEST, content::RESOURCE_TYPE_SCRIPT, 42.),
+      compute_score(net::HIGHEST, content::RESOURCE_TYPE_STYLESHEET, 42.),
+      1e-4);
+
+  // Scripts are more important than fonts and images, and the rest.
+  EXPECT_TRUE(
+      compute_score(net::HIGHEST, content::RESOURCE_TYPE_SCRIPT, 42.) >
+      compute_score(net::HIGHEST, content::RESOURCE_TYPE_FONT_RESOURCE, 42.));
+  EXPECT_TRUE(
+      compute_score(net::HIGHEST, content::RESOURCE_TYPE_FONT_RESOURCE, 42.) >
+      compute_score(net::HIGHEST, content::RESOURCE_TYPE_IMAGE, 42.));
+  EXPECT_TRUE(
+      compute_score(net::HIGHEST, content::RESOURCE_TYPE_FONT_RESOURCE, 42.) >
+      compute_score(net::HIGHEST, content::RESOURCE_TYPE_FAVICON, 42.));
+
+  // All else being equal, position matters.
+  EXPECT_TRUE(compute_score(net::HIGHEST, content::RESOURCE_TYPE_SCRIPT, 12.) >
+              compute_score(net::HIGHEST, content::RESOURCE_TYPE_SCRIPT, 42.));
 }
 
 TEST_F(ResourcePrefetchPredictorTablesTest, GetAllData) {
   TestGetAllData();
 }
 
 TEST_F(ResourcePrefetchPredictorTablesTest, UpdateData) {
   TestUpdateData();
 }
 
@@ skipping 50 matching lines @@
 
 TEST_F(ResourcePrefetchPredictorTablesReopenTest, DeleteSingleDataPoint) {
   TestDeleteSingleDataPoint();
 }
 
 TEST_F(ResourcePrefetchPredictorTablesReopenTest, DeleteAllData) {
   TestDeleteAllData();
 }
 
 }  // namespace predictors