predictors: Basic browsertest checks predictor learning.

chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc

Index: chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
diff --git a/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc b/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..addbe731d6836cd44dd417bb9c9adcc0cc36fdc0
--- /dev/null
+++ b/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
@@ -0,0 +1,221 @@
+// Copyright 2016 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 "base/command_line.h"
+#include "chrome/browser/predictors/resource_prefetch_predictor.h"
+#include "chrome/browser/predictors/resource_prefetch_predictor_factory.h"
+#include "chrome/browser/predictors/resource_prefetch_predictor_test_util.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::UnorderedElementsAreArray;
+
+namespace predictors {
+
+static const char kImageUrl[] = "/predictors/image.png";
+static const char kStyleUrl[] = "/predictors/style.css";
+static const char kScriptUrl[] = "/predictors/script.js";
+static const char kFontUrl[] = "/predictors/font.ttf";
+static const char kHtmlSubresourcesUrl[] = "/predictors/html_subresources.html";
+
+using PageRequestSummary = ResourcePrefetchPredictor::PageRequestSummary;
+using URLRequestSummary = ResourcePrefetchPredictor::URLRequestSummary;
+using net::test_server::HttpRequest;
+using net::test_server::HttpResponse;
+using net::test_server::BasicHttpResponse;
+
+struct ResourceSummary {

[Benoit L, 2016/11/02 14:03:18]

This largely duplicates ResourcePrefetchPredictor::URLRequestSummary.

Is the duplication necessary?
If so, can you align more closely the two classes? (same naming, for instance).
If not, please unify a bit.

[alexilin, 2016/11/02 16:14:47]

These structures have mismatched fields.
ResourceSummary: content, no_store, version
URLRequestSummary: navigation_id, redirect_url

I think the same naming will be best option.

+  ResourceSummary()
+      : resource_type(content::RESOURCE_TYPE_LAST_TYPE),
+        cached(false),
+        has_validators(false),
+        always_revalidate(false),
+        no_store(false),
+        version(0) {}

[Benoit L, 2016/11/02 14:03:18]

Doesn't seem to be used in this CL beyond the Etag (which can be set another
way).
I'm fine with keeping this in this CL if it's going to be used shortly.
Otherwise please remove it.

[alexilin, 2016/11/02 16:14:47]

True
Suggestions? I want to be able to invalidate resource in test.
Well, I don't use all flags in tests yet, but I'm going to write more tests.

+
+  GURL url;
+  content::ResourceType resource_type;
+  std::string mime_type;
+  bool cached;
+  bool has_validators;
+  bool always_revalidate;
+  std::string content;
+  bool no_store;

[Benoit L, 2016/11/02 14:03:18]

tiny nit: I quite like "is_X" for boolean variables.

[alexilin, 2016/11/02 16:14:47]

Done.

+  size_t version;
+};
+
+net::RequestPriority TypeToPriority(content::ResourceType type) {

[Benoit L, 2016/11/02 09:56:42]

I'm not really sure about the mapping of resource type to priorities.
Where do you take this from?

[alexilin, 2016/11/02 10:09:14]

I'm not sure too. Probably it would be better not to check priority at all or
check only when we're expecting prefetch (then it should be net::LOW priority).
I've found mapping in WebKit.
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/fetch/Res...
There is a long way to transform blink::ResourceLoadPriority at first to
blink::WebURLRequest::Priority and then to net::RequestPriority.

[Benoit L, 2016/11/02 12:23:12]

The actual priority logic is indeed more complex. For instance for scripts in
<head>, it's going to be different.
We can probably hardcode the priorities for the various resources in the test
files, with a comment explaining why the value is set this way.

It makes it easier to update whenever the priorities heuristics change as well.
wdyt?

[alexilin, 2016/11/02 13:46:16]

I'm not sure that I understood you correctly because the priorities are already
hardcoded. I suppose you want to define a net priority for each resource
individually. Anyway, PTAL at new patchset.

Could be there any problems with dynamically assigned priorities that will make
tests flaky? I mean, can priorities change from run to run?

+  switch (type) {
+    case content::RESOURCE_TYPE_STYLESHEET:
+    case content::RESOURCE_TYPE_FONT_RESOURCE:
+      return net::HIGHEST;
+    case content::RESOURCE_TYPE_SCRIPT:
+      return net::MEDIUM;
+    case content::RESOURCE_TYPE_IMAGE:
+      return net::LOWEST;
+    default:
+      return net::DEFAULT_PRIORITY;
+  }
+}
+
+// Helper class to track and allow waiting for ResourcePrefetchPredictor events.
+class ResourcePrefetchPredictorTestObserver : public TestObserver {
+ public:
+  explicit ResourcePrefetchPredictorTestObserver(
+      ResourcePrefetchPredictor* predictor,
+      const size_t expected_url_visit_count,
+      const PageRequestSummary& expected_summary)
+      : TestObserver(predictor),
+        message_loop_runner_(new content::MessageLoopRunner),

[Benoit L, 2016/11/02 14:03:18]

nit: content::MessageLoopRunner().
That's safer.

[alexilin, 2016/11/02 16:14:47]

Done.

+        url_visit_count_(expected_url_visit_count),
+        summary_(expected_summary) {}
+
+  ~ResourcePrefetchPredictorTestObserver() override {}
+
+  // ResourcePrefetchPredictor::Observer
+  void OnNavigationLearned(size_t url_visit_count,
+                           const PageRequestSummary& summary) override {
+    EXPECT_EQ(url_visit_count, url_visit_count_);
+    EXPECT_EQ(summary.main_frame_url, summary_.main_frame_url);
+    EXPECT_EQ(summary.initial_url, summary_.initial_url);
+    EXPECT_THAT(summary.subresource_requests,
+                UnorderedElementsAreArray(summary_.subresource_requests));
+    message_loop_runner_->Quit();
+  }
+
+  void Wait() { message_loop_runner_->Run(); }
+
+ private:
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+  size_t url_visit_count_;
+  PageRequestSummary summary_;
+
+  DISALLOW_COPY_AND_ASSIGN(ResourcePrefetchPredictorTestObserver);
+};
+
+class ResourcePrefetchPredictorBrowserTest : public InProcessBrowserTest {
+ public:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitchASCII(
+        switches::kSpeculativeResourcePrefetching,
+        switches::kSpeculativeResourcePrefetchingEnabled);
+  }
+
+  void SetUpOnMainThread() override {
+    embedded_test_server()->RegisterRequestHandler(
+        base::Bind(&ResourcePrefetchPredictorBrowserTest::HandleRequest,
+                   base::Unretained(this)));
+    ASSERT_TRUE(embedded_test_server()->Start());
+    predictor_ =
+        ResourcePrefetchPredictorFactory::GetForProfile(browser()->profile());
+    ASSERT_TRUE(predictor_);
+  }
+
+  void NavigateToURLAndCheckSubresources(
+      const std::string& main_frame_relative) {
+    GURL main_frame_absolute =
+        embedded_test_server()->GetURL(main_frame_relative);
+    std::vector<URLRequestSummary> url_request_summaries;
+    for (const auto& kv : resources_) {
+      if (kv.second.no_store)
+        continue;
+      url_request_summaries.push_back(
+          GetURLRequestSummaryForResource(main_frame_absolute, kv.second));
+    }
+    ResourcePrefetchPredictorTestObserver observer(
+        predictor_, UpdateAndGetVisitCount(main_frame_relative),
+        CreatePageRequestSummary(main_frame_absolute.spec(),
+                                 main_frame_absolute.spec(),
+                                 url_request_summaries));
+    ui_test_utils::NavigateToURL(browser(), main_frame_absolute);
+    observer.Wait();
+  }
+
+  void AddResource(const std::string& relative_url,
+                   content::ResourceType resource_type,
+                   const std::string& mime_type = std::string(),
+                   bool has_validators = true,
+                   bool always_revalidate = false,
+                   const std::string& content = std::string(),
+                   bool no_store = false) {
+    ResourceSummary resource;
+    resource.url = embedded_test_server()->GetURL(relative_url);
+    resource.resource_type = resource_type;
+    resource.mime_type = mime_type;
+    resource.has_validators = has_validators;
+    resource.always_revalidate = always_revalidate;
+    resource.content = content;
+    resource.no_store = no_store;
+    resources_[relative_url] = resource;
+  }
+
+ private:
+  URLRequestSummary GetURLRequestSummaryForResource(
+      const GURL& main_frame_url,
+      const ResourceSummary& summary) {
+    content::WebContents* web_contents =
+        browser()->tab_strip_model()->GetActiveWebContents();
+    int process_id = web_contents->GetRenderProcessHost()->GetID();
+    int frame_id = web_contents->GetMainFrame()->GetRoutingID();
+    return CreateURLRequestSummary(
+        process_id, frame_id, main_frame_url.spec(), summary.url.spec(),
+        summary.resource_type, TypeToPriority(summary.resource_type),
+        summary.mime_type, summary.cached, std::string(),
+        summary.has_validators, summary.always_revalidate);
+  }
+
+  std::unique_ptr<HttpResponse> HandleRequest(const HttpRequest& request) {
+    auto resource_it = resources_.find(request.relative_url);
+    if (resource_it == resources_.end())
+      return nullptr;
+
+    const ResourceSummary& summary = resource_it->second;
+    std::unique_ptr<BasicHttpResponse> http_response(new BasicHttpResponse);

[Benoit L, 2016/11/02 14:03:18]

nit: ()

Or better: MakeUnique

[alexilin, 2016/11/02 16:14:47]

Done.

+    http_response->set_code(net::HTTP_OK);
+    http_response->set_content_type(summary.mime_type);
+    http_response->set_content(summary.content);
+    if (summary.no_store)
+      http_response->AddCustomHeader("Cache-Control", "no-store");

[Benoit L, 2016/11/02 14:03:18]

It's possible to call this several times, good.
Then no_store and has_validators are not mutually exclusive.

[alexilin, 2016/11/02 16:14:47]

Is it good or bad? I had DCHECK checked that no_store and {has_validators,
always_revalidate} are mutually exclusive but I deleted it.
Mix of these headers isn't forbidden in RFC I guess. So we can check the
behavior of the browser in this case.

+    if (summary.has_validators) {
+      http_response->AddCustomHeader(
+          "ETag", base::StringPrintf("'%zu%s'", summary.version,
+                                     request.relative_url.c_str()));
+    }
+    if (summary.always_revalidate)
+      http_response->AddCustomHeader("Cache-Control", "no-cache");
+    else
+      http_response->AddCustomHeader("Cache-Control", "max-age=2147483648");
+    return std::move(http_response);
+  }
+
+  size_t UpdateAndGetVisitCount(const std::string& main_frame_relative) {
+    return ++visit_count_[main_frame_relative];
+  }
+
+  ResourcePrefetchPredictor* predictor_;
+  std::map<std::string, ResourceSummary> resources_;
+  std::map<std::string, size_t> visit_count_;
+};
+
+IN_PROC_BROWSER_TEST_F(ResourcePrefetchPredictorBrowserTest, ) {
+  AddResource(kImageUrl, content::RESOURCE_TYPE_IMAGE, "image/png");
+  AddResource(kStyleUrl, content::RESOURCE_TYPE_STYLESHEET, "text/css");
+  AddResource(kScriptUrl, content::RESOURCE_TYPE_SCRIPT,
+              "application/javascript");
+  AddResource(kFontUrl, content::RESOURCE_TYPE_FONT_RESOURCE,
+              "application/x-font-truetype");
+  NavigateToURLAndCheckSubresources(kHtmlSubresourcesUrl);
+}
+
+}  // namespace predictors