predictors: Take the priority into account for scoring resources.

chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc

Index: chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc
diff --git a/chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc b/chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc
index c969bdea82ca3b0a9bb3bcfb7a65177696bf64a9..f037497399b2d003a44b98e73e08b157cd367175 100644
--- a/chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc
+++ b/chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc
@@ -575,33 +575,47 @@ void ResourcePrefetchPredictorTablesTest::ReopenDatabase() {
 // 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);
-
-  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);
+  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));
+  };
+
+  // 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) {