Change WebUIDataSource::UseGzip() to an exclusion callback (not a list)

This is so callers that need to do dynamic exclusion (print preview) can R=thestig@chromium.org BUG=840024 Change-Id: I27e057a79ee91001e8244ae88a2e267a13236308 Reviewed-on: https://chromium-review.googlesource.com/c/1408032 Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Commit-Queue: Dan Beam <dbeam@chromium.org> Cr-Commit-Position: refs/heads/master@{#622912}
qweri0p · Jan 16, 2019 · a6f790c · a6f790c
1 parent c01da70
commit a6f790c
Show file tree

Hide file tree

Showing 11 changed files with 57 additions and 25 deletions.
diff --git a/chrome/browser/accessibility/accessibility_ui.cc b/chrome/browser/accessibility/accessibility_ui.cc
@@ -239,7 +239,8 @@ AccessibilityUI::AccessibilityUI(content::WebUI* web_ui)
       base::Bind(&HandleAccessibilityRequestCallback,
                  web_ui->GetWebContents()->GetBrowserContext()));
 
-  html_source->UseGzip({kTargetsDataFile});
+  html_source->UseGzip(base::BindRepeating(
+      [](const std::string& path) { return path != kTargetsDataFile; }));
 
   content::BrowserContext* browser_context =
       web_ui->GetWebContents()->GetBrowserContext();

diff --git a/chrome/browser/ui/webui/md_bookmarks/md_bookmarks_ui.cc b/chrome/browser/ui/webui/md_bookmarks/md_bookmarks_ui.cc
@@ -157,7 +157,9 @@ content::WebUIDataSource* CreateMdBookmarksUIHTMLSource(Profile* profile) {
       base::FeatureList::IsEnabled(features::kWebUIPolymer2)
           ? IDR_MD_BOOKMARKS_VULCANIZED_P2_HTML
           : IDR_MD_BOOKMARKS_VULCANIZED_HTML);
-  source->UseGzip({"images/folder_open.svg", "images/folder.svg"});
+  source->UseGzip(base::BindRepeating([](const std::string& path) {
+    return path != "images/folder_open.svg" && path != "images/folder.svg";
+  }));
 #else
   source->AddResourcePath("actions.html", IDR_MD_BOOKMARKS_ACTIONS_HTML);
   source->AddResourcePath("actions.js", IDR_MD_BOOKMARKS_ACTIONS_JS);

diff --git a/chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc b/chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc
@@ -115,9 +115,11 @@ content::WebUIDataSource* CreateDownloadsUIHTMLSource(Profile* profile) {
   source->AddLocalizedString("managedByOrg", IDS_MANAGED_BY_ORG_WITH_HYPERLINK);
 
 #if BUILDFLAG(OPTIMIZE_WEBUI)
-  source->UseGzip({"1x/incognito_marker.png", "1x/no_downloads.png",
-                   "2x/incognito_marker.png", "2x/no_downloads.png",
-                   "md_downloads.mojom-lite.js"});
+  source->UseGzip(base::BindRepeating([](const std::string& path) {
+    return path != "1x/incognito_marker.png" && path != "1x/no_downloads.png" &&
+           path != "2x/incognito_marker.png" && path != "2x/no_downloads.png" &&
+           path != "md_downloads.mojom-lite.js";
+  }));
 
   source->AddResourcePath("crisper.js", IDR_MD_DOWNLOADS_CRISPER_JS);
   source->SetDefaultResource(

diff --git a/chrome/browser/ui/webui/md_history_ui.cc b/chrome/browser/ui/webui/md_history_ui.cc
@@ -12,6 +12,7 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
+#include "base/stl_util.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile.h"
@@ -173,7 +174,12 @@ content::WebUIDataSource* CreateMdHistoryUIHTMLSource(Profile* profile,
     source->AddResourcePath(resource.path, resource.idr);
     exclude_from_gzip.push_back(resource.path);
   }
-  source->UseGzip(exclude_from_gzip);
+  source->UseGzip(base::BindRepeating(
+      [](const std::vector<std::string> excluded_paths,
+         const std::string& path) {
+        return !base::ContainsValue(excluded_paths, path);
+      },
+      std::move(exclude_from_gzip)));
 
 #if BUILDFLAG(OPTIMIZE_WEBUI)
   const bool use_polymer_2 =

diff --git a/chrome/browser/ui/webui/settings/md_settings_ui.cc b/chrome/browser/ui/webui/settings/md_settings_ui.cc
@@ -14,6 +14,7 @@
 #include "ash/public/cpp/ash_features.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/stl_util.h"
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
@@ -400,7 +401,12 @@ MdSettingsUI::MdSettingsUI(content::WebUI* web_ui)
   html_source->SetDefaultResource(use_polymer_2
                                       ? IDR_MD_SETTINGS_VULCANIZED_P2_HTML
                                       : IDR_MD_SETTINGS_VULCANIZED_HTML);
-  html_source->UseGzip(exclude_from_gzip);
+  html_source->UseGzip(base::BindRepeating(
+      [](const std::vector<std::string>& excluded_paths,
+         const std::string& path) {
+        return !base::ContainsValue(excluded_paths, path);
+      },
+      std::move(exclude_from_gzip)));
 #if defined(OS_CHROMEOS)
   html_source->AddResourcePath("manifest.json", IDR_MD_SETTINGS_MANIFEST);
 #endif  // defined (OS_CHROMEOS)

diff --git a/content/browser/tracing/tracing_ui.cc b/content/browser/tracing/tracing_ui.cc
@@ -176,9 +176,12 @@ TracingUI::TracingUI(WebUI* web_ui)
       web_ui->GetWebContents()->GetBrowserContext();
 
   WebUIDataSource* source = WebUIDataSource::Create(kChromeUITracingHost);
-  source->UseGzip({"json/begin_recording", "json/categories",
-                   "json/end_recording_compressed",
-                   "json/get_buffer_percent_full", "json/get_buffer_status"});
+  source->UseGzip(base::BindRepeating([](const std::string& path) {
+    return path != "json/begin_recording" && path != "json/categories" &&
+           path != "json/end_recording_compressed" &&
+           path != "json/get_buffer_percent_full" &&
+           path != "json/get_buffer_status";
+  }));
   source->SetJsonPath("strings.js");
   source->SetDefaultResource(IDR_TRACING_HTML);
   source->AddResourcePath("tracing.js", IDR_TRACING_JS);

diff --git a/content/browser/webui/web_ui_data_source_impl.cc b/content/browser/webui/web_ui_data_source_impl.cc
@@ -169,7 +169,6 @@ void WebUIDataSourceImpl::SetJsonPath(base::StringPiece path) {
   DCHECK(!path.empty());
 
   json_path_ = path.as_string();
-  excluded_paths_.insert(json_path_);
 }
 
 void WebUIDataSourceImpl::AddResourcePath(base::StringPiece path,
@@ -225,10 +224,9 @@ void WebUIDataSourceImpl::UseGzip() {
 }
 
 void WebUIDataSourceImpl::UseGzip(
-    const std::vector<std::string>& excluded_paths) {
+    base::RepeatingCallback<bool(const std::string&)> is_gzipped_callback) {
   UseGzip();
-  for (const auto& path : excluded_paths)
-    excluded_paths_.insert(path);
+  is_gzipped_callback_ = std::move(is_gzipped_callback);
 }
 
 const ui::TemplateReplacements* WebUIDataSourceImpl::GetReplacements() const {
@@ -312,7 +310,15 @@ const base::DictionaryValue* WebUIDataSourceImpl::GetLocalizedStrings() const {
 }
 
 bool WebUIDataSourceImpl::IsGzipped(const std::string& path) const {
-  return use_gzip_ && excluded_paths_.count(CleanUpPath(path)) == 0;
+  if (!use_gzip_)
+    return false;
+
+  // TODO(dbeam): does anybody care about the "dirty" path (i.e. stuff after ?).
+  const std::string clean_path = CleanUpPath(path);
+  if (!json_path_.empty() && clean_path == json_path_)
+    return false;
+
+  return is_gzipped_callback_.is_null() || is_gzipped_callback_.Run(clean_path);
 }
 
 }  // namespace content
diff --git a/content/browser/webui/web_ui_data_source_impl.h b/content/browser/webui/web_ui_data_source_impl.h
@@ -9,7 +9,6 @@
 
 #include <map>
 #include <string>
-#include <unordered_set>
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
@@ -50,7 +49,8 @@ class CONTENT_EXPORT WebUIDataSourceImpl : public URLDataSourceImpl,
   void OverrideContentSecurityPolicyChildSrc(const std::string& data) override;
   void DisableDenyXFrameOptions() override;
   void UseGzip() override;
-  void UseGzip(const std::vector<std::string>& excluded_paths) override;
+  void UseGzip(base::RepeatingCallback<bool(const std::string&)>
+                   is_gzipped_callback) override;
   std::string GetSource() const override;
 
   // URLDataSourceImpl:
@@ -79,7 +79,7 @@ class CONTENT_EXPORT WebUIDataSourceImpl : public URLDataSourceImpl,
   friend class WebUIDataSourceTest;
 
   FRIEND_TEST_ALL_PREFIXES(WebUIDataSourceTest, IsGzipped);
-  FRIEND_TEST_ALL_PREFIXES(WebUIDataSourceTest, IsGzippedWithExclusions);
+  FRIEND_TEST_ALL_PREFIXES(WebUIDataSourceTest, IsGzippedWithCallback);
 
   // Methods that match URLDataSource which are called by
   // InternalDataSource.
@@ -103,7 +103,6 @@ class CONTENT_EXPORT WebUIDataSourceImpl : public URLDataSourceImpl,
   int default_resource_;
   std::string json_path_;
   std::map<std::string, int> path_to_idr_map_;
-  std::unordered_set<std::string> excluded_paths_;
   // The replacements are initiallized in the main thread and then used in the
   // IO thread. The map is safe to read from multiple threads as long as no
   // futher changes are made to it after initialization.
@@ -124,6 +123,7 @@ class CONTENT_EXPORT WebUIDataSourceImpl : public URLDataSourceImpl,
   bool add_load_time_data_defaults_;
   bool replace_existing_source_;
   bool use_gzip_;
+  base::RepeatingCallback<bool(const std::string&)> is_gzipped_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(WebUIDataSourceImpl);
 };

diff --git a/content/browser/webui/web_ui_data_source_unittest.cc b/content/browser/webui/web_ui_data_source_unittest.cc
@@ -214,17 +214,20 @@ TEST_F(WebUIDataSourceTest, MimeType) {
 
 TEST_F(WebUIDataSourceTest, IsGzipped) {
   EXPECT_FALSE(source()->IsGzipped("foobar"));
+  EXPECT_FALSE(source()->IsGzipped(""));
   source()->UseGzip();
   EXPECT_TRUE(source()->IsGzipped("foobar"));
+  EXPECT_TRUE(source()->IsGzipped(""));
 }
 
-TEST_F(WebUIDataSourceTest, IsGzippedWithExclusions) {
+TEST_F(WebUIDataSourceTest, IsGzippedWithCallback) {
   EXPECT_FALSE(source()->IsGzipped("foobar"));
 
   source()->AddResourcePath("foobar", kDummyResourceId);
   source()->SetDefaultResource(kDummyDefaultResourceId);
   source()->SetJsonPath("strings.js");
-  source()->UseGzip({"json/special/path"});
+  source()->UseGzip(base::BindRepeating(
+      [](const std::string& path) { return path != "json/special/path"; }));
 
   EXPECT_TRUE(source()->IsGzipped("foobar"));
   EXPECT_TRUE(source()->IsGzipped("foobar?query"));

diff --git a/content/public/browser/web_ui_data_source.h b/content/public/browser/web_ui_data_source.h
@@ -108,10 +108,13 @@ class WebUIDataSource {
       const std::string& data) = 0;
   virtual void DisableDenyXFrameOptions() = 0;
 
-  // Tells the loading code that resources are gzipped on disk. |excluded_paths|
-  // are uncompressed paths, and therefore should not be decompressed.
+  // Tells the loading code that resources are gzipped on disk.
   virtual void UseGzip() = 0;
-  virtual void UseGzip(const std::vector<std::string>& excluded_paths) = 0;
+
+  // Same as UseGzip() above, but |is_gzipped_callback| is used to dynamically
+  // determine whether a given |const std::string& path| argument is gzipped.
+  virtual void UseGzip(base::RepeatingCallback<bool(const std::string&)>
+                           is_gzipped_callback) = 0;
 
   // The |source_name| this WebUIDataSource was created with.
   virtual std::string GetSource() const = 0;

diff --git a/docs/optimizing_web_uis.md b/docs/optimizing_web_uis.md
@@ -120,5 +120,5 @@ To mark a WebUI's resources compressed, you'll need to do something like:
 ```c++
 WebUIDataSource* data_source = WebUIDataSource::Create(...);
 data_source->SetDefaultResource(IDR_MY_PAGE);
-data_source->UseGzip({"strings.js", ...});  // Omit arg to compress everything
+data_source->UseGzip();  // Optional callback if exclusions are needed.
 ```