Lower priority for JavaScript requests when defer intervention is enabled

Change-Id: I3ecf89469653e98afed23f4d6cf8185bde032be8
Bug: 978490
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1687483
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676141}

chrome/browser/previews/defer_all_script_priority_browsertest.cc

+// Copyright 2019 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 <string>
+
+#include "base/bind.h"
+#include "base/command_line.h"
+#include "base/logging.h"
+#include "base/macros.h"
+#include "base/run_loop.h"
+#include "base/test/metrics/histogram_tester.h"
+#include "base/test/scoped_feature_list.h"
+#include "build/build_config.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/browser_process_impl.h"
+#include "chrome/browser/chrome_content_browser_client.h"
+#include "chrome/browser/metrics/subprocess_metrics_provider.h"
+#include "chrome/browser/previews/previews_service.h"
+#include "chrome/browser/previews/previews_service_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "components/data_reduction_proxy/core/common/data_reduction_proxy_features.h"
+#include "components/optimization_guide/hints_component_info.h"
+#include "components/optimization_guide/optimization_guide_features.h"
+#include "components/optimization_guide/optimization_guide_service.h"
+#include "components/optimization_guide/proto/hints.pb.h"
+#include "components/optimization_guide/test_hints_component_creator.h"
+#include "components/previews/content/previews_hints.h"
+#include "components/previews/content/previews_optimization_guide.h"
+#include "components/previews/content/previews_ui_service.h"
+#include "components/previews/core/previews_constants.h"
+#include "components/previews/core/previews_features.h"
+#include "components/previews/core/previews_switches.h"
+#include "content/public/test/browser_test.h"
+#include "content/public/test/browser_test_base.h"
+#include "content/public/test/browser_test_utils.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
+
+namespace {
+
+// Retries fetching |histogram_name| until it contains at least |count| samples.
+void RetryForHistogramUntilCountReached(base::HistogramTester* histogram_tester,
+                                        const std::string& histogram_name,
+                                        size_t count) {
+  while (true) {
+    base::ThreadPoolInstance::Get()->FlushForTesting();
+    base::RunLoop().RunUntilIdle();
+
+    content::FetchHistogramsFromChildProcesses();
+    SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
+
+    const std::vector<base::Bucket> buckets =
+        histogram_tester->GetAllSamples(histogram_name);
+    size_t total_count = 0;
+    for (const auto& bucket : buckets) {
+      total_count += bucket.count;
+    }
+    if (total_count >= count) {
+      break;
+    }
+  }
+}
+
+}  // namespace
+
+class DeferAllScriptPriorityBrowserTest
+    : public ::testing::WithParamInterface<bool>,
+      public InProcessBrowserTest {
+ public:
+  DeferAllScriptPriorityBrowserTest() = default;
+  ~DeferAllScriptPriorityBrowserTest() override = default;
+
+  void SetUp() override {
+    if (IsDeferAllScriptFeatureEnabled()) {
+      scoped_feature_list_.InitWithFeatures(
+          {previews::features::kPreviews,
+           previews::features::kDeferAllScriptPreviews,
+           optimization_guide::features::kOptimizationHints,
+           data_reduction_proxy::features::
+               kDataReductionProxyEnabledWithNetworkService},
+          {});
+    } else {
+      scoped_feature_list_.InitWithFeatures(
+          {previews::features::kPreviews,
+           optimization_guide::features::kOptimizationHints,
+           data_reduction_proxy::features::
+               kDataReductionProxyEnabledWithNetworkService},
+          {});
+    }
+
+    InProcessBrowserTest::SetUp();
+  }
+
+  bool IsDeferAllScriptFeatureEnabled() const { return GetParam(); }
+
+  // Returns the fetch time for the JavaScript file (in milliseconds). This
+  // value is obtained using the resource timing API.
+  double GetFetchTimeForJavaScriptFileInMilliseconds() {
+    double script_log;
+    std::string script = "getFetchTimeForJavaScriptFileInMilliseconds()";
+    EXPECT_TRUE(ExecuteScriptAndExtractDouble(
+        browser()->tab_strip_model()->GetActiveWebContents(), script,
+        &script_log));
+    return script_log;
+  }
+
+  void SetUpOnMainThread() override {
+    https_server_.reset(
+        new net::EmbeddedTestServer(net::EmbeddedTestServer::TYPE_HTTPS));
+    https_server_->RegisterRequestHandler(
+        base::BindRepeating(&DeferAllScriptPriorityBrowserTest::RequestHandler,
+                            base::Unretained(this)));
+
+    ASSERT_TRUE(https_server_->Start());
+
+    https_url_ = https_server_->GetURL("/defer_all_script_priority_test.html");
+    ASSERT_TRUE(https_url_.SchemeIs(url::kHttpsScheme));
+
+    InProcessBrowserTest::SetUpOnMainThread();
+  }
+
+  void SetUpCommandLine(base::CommandLine* cmd) override {
+    cmd->AppendSwitch("enable-spdy-proxy-auth");
+    cmd->AppendSwitchASCII("--force-effective-connection-type", "Slow-2G");
+
+    cmd->AppendSwitch("optimization-guide-disable-installer");
+    cmd->AppendSwitch("purge_hint_cache_store");
+
+    // Due to race conditions, it's possible that blacklist data is not loaded
+    // at the time of first navigation. That may prevent Preview from
+    // triggering, and causing the test to flake.
+    cmd->AppendSwitch(previews::switches::kIgnorePreviewsBlacklist);
+  }
+
+  // Returns a unique script for each request, to test service worker update.
+  std::unique_ptr<net::test_server::HttpResponse> RequestHandler(
+      const net::test_server::HttpRequest& request) {
+    if (request.GetURL().path().find("/defer_all_script_priority_test.html") !=
+        std::string::npos) {
+      return GetHtmlResponse(request);
+    }
+
+    if (request.GetURL().path().find("/hung") != std::string::npos) {
+      return GetDelayedResponse(request);
+    }
+    return std::make_unique<net::test_server::BasicHttpResponse>();
+  }
+
+  // Returns an HTML response that fetches CSS files followed by synchronous
+  // external JavaScript file. The HTML file contains an inline JavaScript
+  // function getFetchTimeForJavaScriptFileInMilliseconds() that returns
+  // the fetch time for JavaScript file in milliseconds.
+  std::unique_ptr<net::test_server::HttpResponse> GetHtmlResponse(
+      const net::test_server::HttpRequest& request) {
+    std::unique_ptr<net::test_server::BasicHttpResponse> response =
+        std::make_unique<net::test_server::BasicHttpResponse>();
+    response->set_content(
+        "<html><body>"
+        "<link rel=\"stylesheet\" type=\"text/css\" href=\"hung1.css\">"
+        "<link rel=\"stylesheet\" type=\"text/css\" href=\"hung2.css\">"
+        "<link rel=\"stylesheet\" type=\"text/css\" href=\"hung3.css\">"
+        "<link rel=\"stylesheet\" type=\"text/css\" href=\"hung4.css\">"
+        "<script src=\"defer_all_script_syncscript.js\"></script>"
+        "<script>function getFetchTimeForJavaScriptFileInMilliseconds() {"
+        "var p=window.performance.getEntriesByType(\"resource\");"
+        "for (var i=0; i < p.length; i++) {"
+        "if(p[i].name.includes(\"defer_all_script_syncscript.js\")) {"
+        "sendValueToTest(p[i].responseStart-p[i].fetchStart);"
+        "}"
+        "}"
+        "}"
+        "function sendValueToTest(value) {"
+        "window.domAutomationController.send(value);"
+        "}"
+        "</script></body></html>");
+    return std::move(response);
+  }
+
+  int css_files_hung_time_milliseconds() { return 5000; }
+
+  std::unique_ptr<net::test_server::HttpResponse> GetDelayedResponse(
+      const net::test_server::HttpRequest& request) {
+    std::unique_ptr<net::test_server::DelayedHttpResponse> response =
+        std::make_unique<net::test_server::DelayedHttpResponse>(
+            base::TimeDelta::FromMilliseconds(
+                css_files_hung_time_milliseconds()));
+    response->set_code(net::HttpStatusCode::HTTP_OK);
+    return std::move(response);
+  }
+
+  // Creates hint data from the |component_info| and waits for it to be fully
+  // processed before returning.
+  void ProcessHintsComponent(
+      const optimization_guide::HintsComponentInfo& component_info) {
+    // Register a QuitClosure for when the next hint update is started below.
+    base::RunLoop run_loop;
+    PreviewsServiceFactory::GetForProfile(
+        Profile::FromBrowserContext(browser()
+                                        ->tab_strip_model()
+                                        ->GetActiveWebContents()
+                                        ->GetBrowserContext()))
+        ->previews_ui_service()
+        ->previews_decider_impl()
+        ->previews_opt_guide()
+        ->ListenForNextUpdateForTesting(run_loop.QuitClosure());
+
+    g_browser_process->optimization_guide_service()->MaybeUpdateHintsComponent(
+        component_info);
+    run_loop.Run();
+  }
+
+  // Performs a navigation to |url| and waits for the the url's host's hints to
+  // load before returning. This ensures that the hints will be available in the
+  // hint cache for a subsequent navigation to a test url with the same host.
+  void LoadHintsForUrl(const GURL& url) {
+    base::HistogramTester histogram_tester;
+
+    // Navigate to the url to prime the OptimizationGuide hints for the
+    // url's host and ensure that they have been loaded from the store (via
+    // histogram) prior to the navigation that tests functionality.
+    ui_test_utils::NavigateToURL(browser(), url);
+
+    RetryForHistogramUntilCountReached(
+        &histogram_tester,
+        previews::kPreviewsOptimizationGuideOnLoadedHintResultHistogramString,
+        1);
+  }
+
+  void SetDeferAllScriptHintWithPageWithPattern(
+      const GURL& hint_setup_url,
+      const std::string& page_pattern) {
+    ProcessHintsComponent(
+        test_hints_component_creator_.CreateHintsComponentInfoWithPageHints(
+            optimization_guide::proto::DEFER_ALL_SCRIPT,
+            {hint_setup_url.host()}, page_pattern, {}));
+    LoadHintsForUrl(hint_setup_url);
+  }
+
+  virtual const GURL& https_url() const { return https_url_; }
+
+ protected:
+  base::test::ScopedFeatureList scoped_feature_list_;
+
+ private:
+  void TearDownOnMainThread() override {
+    EXPECT_TRUE(https_server_->ShutdownAndWaitUntilComplete());
+
+    InProcessBrowserTest::TearDownOnMainThread();
+  }
+
+  optimization_guide::testing::TestHintsComponentCreator
+      test_hints_component_creator_;
+
+  std::unique_ptr<net::EmbeddedTestServer> https_server_;
+  GURL https_url_;
+
+  DISALLOW_COPY_AND_ASSIGN(DeferAllScriptPriorityBrowserTest);
+};
+
+// Parameter is true if the test should be run with defer feature enabled.
+INSTANTIATE_TEST_SUITE_P(,
+                         DeferAllScriptPriorityBrowserTest,
+                         ::testing::Values(false, true));
+
+// Avoid flakes and issues on non-applicable platforms.
+#if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_CHROMEOS)
+#define DISABLE_ON_WIN_MAC_CHROMESOS(x) DISABLED_##x
+#else
+#define DISABLE_ON_WIN_MAC_CHROMESOS(x) x
+#endif
+
+// Fetches an HTML weboage that fetches CSS files followed by an external
+// JavaScript file. Verifies that the fetching of the JavaScript files is
+// delayed only when it's not render blocking.
+//
+// When feature kDeferAllScriptPreviews is enabled, loading priority of
+// JavaScript file should be lowered. This should cause resource scheduler
+// to mark the JavaScript request as delayable, and delay its fetching to after
+// the fetching of CSS files finish.
+//
+// However, if kDeferAllScriptPreviews is not enabled, then JavaScript is render
+// blocking. This should cause resource scheduler to mark the JavaScript request
+// as non-delayale, thus fetching it in parallel with the CSS files.
+IN_PROC_BROWSER_TEST_P(
+    DeferAllScriptPriorityBrowserTest,
+    DISABLE_ON_WIN_MAC_CHROMESOS(DeferAllScriptHttpsWhitelisted)) {
+  GURL url = https_url();
+
+  if (IsDeferAllScriptFeatureEnabled()) {
+    // Whitelist DeferAllScript for any path for the url's host.
+    SetDeferAllScriptHintWithPageWithPattern(url, "*");
+  }
+
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  double delay_milliseconds = GetFetchTimeForJavaScriptFileInMilliseconds();
+
+  if (IsDeferAllScriptFeatureEnabled()) {
+    // Fetching of JavaScript must start after fetching of the CSS files are
+    // finished.
+    EXPECT_LT(css_files_hung_time_milliseconds(), delay_milliseconds);
+  } else {
+    // Fetching of JavaScript should start in parallel with fetching of the
+    // other files. So, it should finish fast enough. Note that even without any
+    // queuing delays in resource scheduler, it's possible that this fetching
+    // takes more css_files_hung_time_milliseconds(). This can potentially make
+    // this test a bit flaky.
+    EXPECT_GT(css_files_hung_time_milliseconds(), delay_milliseconds);
+  }
+}

chrome/test/BUILD.gn

@@ -1015,6 +1015,7 @@ if (!is_android) {
       "../browser/prerender/prerender_test_utils.cc",
       "../browser/prerender/prerender_test_utils.h",
       "../browser/previews/defer_all_script_browsertest.cc",
+      "../browser/previews/defer_all_script_priority_browsertest.cc",
       "../browser/previews/hints_fetcher_browsertest.cc",
       "../browser/previews/lazyload_browsertest.cc",
       "../browser/previews/previews_browsertest.cc",

third_party/blink/renderer/core/loader/document_loader.h

@@ -282,6 +282,10 @@ class CORE_EXPORT DocumentLoader
 
   void SetLoadingJavaScriptUrl() { loading_url_as_javascript_ = true; }
 
+  WebURLRequest::PreviewsState previews_state() const {
+    return previews_state_;
+  }
+
  protected:
   bool had_transient_activation() const { return had_transient_activation_; }
 

third_party/blink/renderer/core/loader/frame_fetch_context.cc

@@ -323,6 +323,10 @@ FrameFetchContext::GetPreviewsResourceLoadingHints() const {
   return document_loader->GetPreviewsResourceLoadingHints();
 }
 
+WebURLRequest::PreviewsState FrameFetchContext::previews_state() const {
+  return GetLocalFrameClient()->GetPreviewsStateForFrame();
+}
+
 LocalFrame* FrameFetchContext::GetFrame() const {
   return &frame_or_imported_document_->GetFrame();
 }

third_party/blink/renderer/core/loader/frame_fetch_context.h

@@ -130,6 +130,7 @@ class CORE_EXPORT FrameFetchContext final : public BaseFetchContext {
   SubresourceFilter* GetSubresourceFilter() const override;
   PreviewsResourceLoadingHints* GetPreviewsResourceLoadingHints()
       const override;
+  WebURLRequest::PreviewsState previews_state() const override;
   bool AllowScriptFromSource(const KURL&) const override;
   bool ShouldBlockRequestByInspector(const KURL&) const override;
   void DispatchDidBlockRequest(const ResourceRequest&,

third_party/blink/renderer/platform/loader/fetch/fetch_context.h

@@ -142,6 +142,10 @@ class PLATFORM_EXPORT FetchContext
     return false;
   }
 
+  virtual WebURLRequest::PreviewsState previews_state() const {
+    return WebURLRequest::kPreviewsUnspecified;
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(FetchContext);
 };

third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc

@@ -243,6 +243,33 @@ ResourceLoadPriority AdjustPriorityWithPriorityHint(
   return new_priority;
 }
 
+ResourceLoadPriority AdjustPriorityWithDeferScriptIntervention(
+    const FetchContext& fetch_context,
+    ResourceLoadPriority priority_so_far,
+    ResourceType type,
+    const ResourceRequest& resource_request,
+    FetchParameters::DeferOption defer_option,
+    bool is_link_preload) {
+  WebURLRequest::PreviewsState context_previews_state =
+      fetch_context.previews_state();
+
+  if (type != ResourceType::kScript)
+    return priority_so_far;
+
+  // If none of the JavaScript resources are render blocking (due to the
+  // DeferAllScript intervention), then lower their priority so they do not
+  // contend for network resources with higher priority resources that may be
+  // render blocking (e.g., html, css). ResourceLoadPriority::kMedium
+  // corresponds to a network priority of
+  // network::mojom::blink::RequestPriority::kLow which is considered delayable
+  // by the resource scheduler on the browser side.
+  if (RuntimeEnabledFeatures::ForceDeferScriptInterventionEnabled() ||
+      (context_previews_state & WebURLRequest::kDeferAllScriptOn)) {
+    return std::min(priority_so_far, ResourceLoadPriority::kMedium);
+  }
+  return priority_so_far;
+}
+
 std::unique_ptr<TracedValue> BeginResourceLoadData(
     const blink::ResourceRequest& request) {
   auto value = std::make_unique<TracedValue>();
@@ -508,6 +535,10 @@ ResourceLoadPriority ResourceFetcher::ComputeLoadPriority(
   priority = AdjustPriorityWithPriorityHint(priority, type, resource_request,
                                             defer_option, is_link_preload);
 
+  priority = AdjustPriorityWithDeferScriptIntervention(
+      Context(), priority, type, resource_request, defer_option,
+      is_link_preload);
+
   if (properties_->IsSubframeDeprioritizationEnabled()) {
     if (properties_->IsMainFrame()) {
       DEFINE_STATIC_LOCAL(