Revert "DNR: Save indexed ruleset checksum as part of Extension prefe…

…rences." This reverts commit 63b0d51. Reason for revert: Speculative revert for the MSan failures on Linux for including a lot of Extension tests. See first failure here: https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20MSan%20Tests/builds/4885 Original change's description: > DNR: Save indexed ruleset checksum as part of Extension preferences. > > This CL adds a new extension preference "dnr_ruleset_checksum" to save the > checksum of the indexed ruleset for the Declarative Net Request API. This > preference is saved during extension installation for packed and loading for > unpacked extensions. The checksum is useful to determine whether an indexed > ruleset exists for an extension and to verify the integrity of the ruleset on > disk. > > Doc=http://go/declarative-net-request (Internal only) > BUG=696822 > > Change-Id: I092cc9c113a7ba61562314462f8fcd74bd8a8b11 > Reviewed-on: https://chromium-review.googlesource.com/673829 > Commit-Queue: Karan Bhatia <karandeepb@chromium.org> > Reviewed-by: Scott Violet <sky@chromium.org> > Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> > Cr-Commit-Position: refs/heads/master@{#507510} TBR=sky@chromium.org,lazyboy@chromium.org,karandeepb@chromium.org Change-Id: Ia33c9729f8ba6c27336d0aae7a5d39d94c448a4a No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 696822 Reviewed-on: https://chromium-review.googlesource.com/709494 Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org> Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org> Cr-Commit-Position: refs/heads/master@{#507666}
FairyWorld · Oct 10, 2017 · 6f31554 · 6f31554
1 parent 327bb21
commit 6f31554
Show file tree

Hide file tree

Showing 23 changed files with 113 additions and 241 deletions.
diff --git a/chrome/browser/chromeos/app_mode/kiosk_app_data.cc b/chrome/browser/chromeos/app_mode/kiosk_app_data.cc
@@ -99,13 +99,11 @@ class KioskAppData::CrxLoader : public extensions::SandboxedUnpackerClient {
   ~CrxLoader() override {}
 
   // extensions::SandboxedUnpackerClient
-  void OnUnpackSuccess(
-      const base::FilePath& temp_dir,
-      const base::FilePath& extension_root,
-      std::unique_ptr<base::DictionaryValue> original_manifest,
-      const extensions::Extension* extension,
-      const SkBitmap& install_icon,
-      const base::Optional<int>& dnr_ruleset_checksum) override {
+  void OnUnpackSuccess(const base::FilePath& temp_dir,
+                       const base::FilePath& extension_root,
+                       std::unique_ptr<base::DictionaryValue> original_manifest,
+                       const extensions::Extension* extension,
+                       const SkBitmap& install_icon) override {
     DCHECK(task_runner_->RunsTasksInCurrentSequence());
 
     const extensions::KioskModeInfo* info =

diff --git a/chrome/browser/chromeos/app_mode/kiosk_external_update_validator.cc b/chrome/browser/chromeos/app_mode/kiosk_external_update_validator.cc
@@ -55,8 +55,7 @@ void KioskExternalUpdateValidator::OnUnpackSuccess(
     const base::FilePath& extension_dir,
     std::unique_ptr<base::DictionaryValue> original_manifest,
     const extensions::Extension* extension,
-    const SkBitmap& install_icon,
-    const base::Optional<int>& dnr_ruleset_checksum) {
+    const SkBitmap& install_icon) {
   DCHECK(crx_file_.extension_id == extension->id());
 
   std::string minimum_browser_version;

diff --git a/chrome/browser/chromeos/app_mode/kiosk_external_update_validator.h b/chrome/browser/chromeos/app_mode/kiosk_external_update_validator.h
@@ -53,13 +53,11 @@ class KioskExternalUpdateValidator
 
   // SandboxedUnpackerClient overrides.
   void OnUnpackFailure(const extensions::CrxInstallError& error) override;
-  void OnUnpackSuccess(
-      const base::FilePath& temp_dir,
-      const base::FilePath& extension_dir,
-      std::unique_ptr<base::DictionaryValue> original_manifest,
-      const extensions::Extension* extension,
-      const SkBitmap& install_icon,
-      const base::Optional<int>& dnr_ruleset_checksum) override;
+  void OnUnpackSuccess(const base::FilePath& temp_dir,
+                       const base::FilePath& extension_dir,
+                       std::unique_ptr<base::DictionaryValue> original_manifest,
+                       const extensions::Extension* extension,
+                       const SkBitmap& install_icon) override;
 
   // Task runner for executing file I/O tasks.
   const scoped_refptr<base::SequencedTaskRunner> backend_task_runner_;

diff --git a/chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc b/chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc
@@ -63,7 +63,7 @@ class DeclarativeNetRequestBrowserTest
     }
     ASSERT_TRUE(extension);
 
-    EXPECT_TRUE(HasValidIndexedRuleset(*extension, profile()));
+    EXPECT_TRUE(HasValidIndexedRuleset(*extension));
 
     int expected_histogram_count = 1;
     tester.ExpectTotalCount(kIndexRulesTimeHistogram, expected_histogram_count);

diff --git a/chrome/browser/extensions/api/declarative_net_request/rule_indexing_unittest.cc b/chrome/browser/extensions/api/declarative_net_request/rule_indexing_unittest.cc
@@ -16,7 +16,6 @@
 #include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_service_test_base.h"
-#include "chrome/browser/profiles/profile.h"
 #include "components/version_info/version_info.h"
 #include "extensions/browser/api/declarative_net_request/constants.h"
 #include "extensions/browser/api/declarative_net_request/parse_info.h"
@@ -95,7 +94,7 @@ class RuleIndexingTest
     extension_ = loader_->LoadExtension(extension_dir_);
     ASSERT_TRUE(extension_.get());
 
-    EXPECT_TRUE(HasValidIndexedRuleset(*extension_, profile()));
+    EXPECT_TRUE(HasValidIndexedRuleset(*extension_));
 
     // Ensure no load errors were reported.
     EXPECT_TRUE(error_reporter()->GetErrors()->empty());

diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
@@ -219,7 +219,7 @@ void CrxInstaller::ConvertUserScriptOnFileThread() {
   }
 
   OnUnpackSuccess(extension->path(), extension->path(), nullptr,
-                  extension.get(), SkBitmap(), base::nullopt);
+                  extension.get(), SkBitmap());
 }
 
 void CrxInstaller::InstallWebApp(const WebApplicationInfo& web_app) {
@@ -244,7 +244,7 @@ void CrxInstaller::ConvertWebAppOnFileThread(
   // TODO(aa): conversion data gets lost here :(
 
   OnUnpackSuccess(extension->path(), extension->path(), nullptr,
-                  extension.get(), SkBitmap(), base::nullopt);
+                  extension.get(), SkBitmap());
 }
 
 CrxInstallError CrxInstaller::AllowInstall(const Extension* extension) {
@@ -423,8 +423,7 @@ void CrxInstaller::OnUnpackSuccess(
     const base::FilePath& extension_dir,
     std::unique_ptr<base::DictionaryValue> original_manifest,
     const Extension* extension,
-    const SkBitmap& install_icon,
-    const base::Optional<int>& dnr_ruleset_checksum) {
+    const SkBitmap& install_icon) {
   DCHECK(installer_task_runner_->RunsTasksInCurrentSequence());
 
   UMA_HISTOGRAM_ENUMERATION("Extensions.UnpackSuccessInstallSource",
@@ -437,8 +436,6 @@ void CrxInstaller::OnUnpackSuccess(
 
   extension_ = extension;
   temp_dir_ = temp_dir;
-  dnr_ruleset_checksum_ = dnr_ruleset_checksum;
-
   if (!install_icon.empty())
     install_icon_.reset(new SkBitmap(install_icon));
 
@@ -835,8 +832,8 @@ void CrxInstaller::ReportSuccessFromUIThread() {
     }
   }
 
-  service_weak_->OnExtensionInstalled(extension(), page_ordinal_,
-                                      install_flags_, dnr_ruleset_checksum_);
+  service_weak_->OnExtensionInstalled(
+      extension(), page_ordinal_, install_flags_);
   NotifyCrxInstallComplete(true);
 }
 

diff --git a/chrome/browser/extensions/crx_installer.h b/chrome/browser/extensions/crx_installer.h
@@ -14,7 +14,6 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
-#include "base/optional.h"
 #include "base/version.h"
 #include "chrome/browser/extensions/extension_install_prompt.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -232,13 +231,11 @@ class CrxInstaller : public SandboxedUnpackerClient {
 
   // SandboxedUnpackerClient
   void OnUnpackFailure(const CrxInstallError& error) override;
-  void OnUnpackSuccess(
-      const base::FilePath& temp_dir,
-      const base::FilePath& extension_dir,
-      std::unique_ptr<base::DictionaryValue> original_manifest,
-      const Extension* extension,
-      const SkBitmap& install_icon,
-      const base::Optional<int>& dnr_ruleset_checksum) override;
+  void OnUnpackSuccess(const base::FilePath& temp_dir,
+                       const base::FilePath& extension_dir,
+                       std::unique_ptr<base::DictionaryValue> original_manifest,
+                       const Extension* extension,
+                       const SkBitmap& install_icon) override;
 
   // Called on the UI thread to start the requirements, policy and blacklist
   // checks on the extension.
@@ -443,10 +440,6 @@ class CrxInstaller : public SandboxedUnpackerClient {
   // The flags for ExtensionService::OnExtensionInstalled.
   int install_flags_;
 
-  // The checksum for the indexed ruleset corresponding to the Declarative Net
-  // Request API.
-  base::Optional<int> dnr_ruleset_checksum_;
-
   // Checks that may run before installing the extension.
   std::unique_ptr<PreloadCheck> policy_check_;
   std::unique_ptr<PreloadCheck> requirements_check_;

diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
@@ -1404,11 +1404,11 @@ void ExtensionService::AddComponentExtension(const Extension* extension) {
         << extension->id() << ") installing/upgrading from '"
         << old_version_string << "' to " << extension->version()->GetString();
 
-    // TODO(crbug.com/696822): If needed, add support for Declarative Net
-    // Request to component extensions and pass the ruleset checksum here.
-    AddNewOrUpdatedExtension(
-        extension, Extension::ENABLED, extensions::kInstallFlagNone,
-        syncer::StringOrdinal(), std::string(), base::nullopt);
+    AddNewOrUpdatedExtension(extension,
+                             Extension::ENABLED,
+                             extensions::kInstallFlagNone,
+                             syncer::StringOrdinal(),
+                             std::string());
     return;
   }
 
@@ -1574,8 +1574,7 @@ void ExtensionService::UpdateActiveExtensionsInCrashReporter() {
 void ExtensionService::OnExtensionInstalled(
     const Extension* extension,
     const syncer::StringOrdinal& page_ordinal,
-    int install_flags,
-    const base::Optional<int>& dnr_ruleset_checksum) {
+    int install_flags) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   const std::string& id = extension->id();
@@ -1683,13 +1682,12 @@ void ExtensionService::OnExtensionInstalled(
   switch (action) {
     case extensions::InstallGate::INSTALL:
       AddNewOrUpdatedExtension(extension, initial_state, install_flags,
-                               page_ordinal, install_parameter,
-                               dnr_ruleset_checksum);
+                               page_ordinal, install_parameter);
       return;
     case extensions::InstallGate::DELAY:
-      extension_prefs_->SetDelayedInstallInfo(
-          extension, initial_state, install_flags, delay_reason, page_ordinal,
-          install_parameter, dnr_ruleset_checksum);
+      extension_prefs_->SetDelayedInstallInfo(extension, initial_state,
+                                              install_flags, delay_reason,
+                                              page_ordinal, install_parameter);
 
       // Transfer ownership of |extension|.
       delayed_installs_.Insert(extension);
@@ -1738,12 +1736,10 @@ void ExtensionService::AddNewOrUpdatedExtension(
     Extension::State initial_state,
     int install_flags,
     const syncer::StringOrdinal& page_ordinal,
-    const std::string& install_parameter,
-    const base::Optional<int>& dnr_ruleset_checksum) {
+    const std::string& install_parameter) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  extension_prefs_->OnExtensionInstalled(extension, initial_state, page_ordinal,
-                                         install_flags, install_parameter,
-                                         dnr_ruleset_checksum);
+  extension_prefs_->OnExtensionInstalled(
+      extension, initial_state, page_ordinal, install_flags, install_parameter);
   delayed_installs_.Remove(extension->id());
   if (InstallVerifier::NeedsVerification(*extension))
     InstallVerifier::Get(GetBrowserContext())->VerifyExtension(extension->id());

diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
@@ -17,7 +17,6 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
-#include "base/optional.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/extensions/blacklist.h"
 #include "chrome/browser/extensions/extension_management.h"
@@ -319,16 +318,12 @@ class ExtensionService
 
   // Informs the service that an extension's files are in place for loading.
   //
-  // |extension|            the extension
-  // |page_ordinal|         the location of the extension in the app launcher
-  // |install_flags|        a bitmask of extensions::InstallFlags
-  // |dnr_ruleset_checksum| Checksum of the indexed ruleset for the Declarative
-  //                        Net Request API.
-  void OnExtensionInstalled(
-      const extensions::Extension* extension,
-      const syncer::StringOrdinal& page_ordinal,
-      int install_flags,
-      const base::Optional<int>& dnr_ruleset_checksum = base::nullopt);
+  // |extension|     the extension
+  // |page_ordinal|  the location of the extension in the app launcher
+  // |install_flags| a bitmask of extensions::InstallFlags
+  void OnExtensionInstalled(const extensions::Extension* extension,
+                            const syncer::StringOrdinal& page_ordinal,
+                            int install_flags);
   void OnExtensionInstalled(const extensions::Extension* extension,
                             const syncer::StringOrdinal& page_ordinal) {
     OnExtensionInstalled(extension,
@@ -523,13 +518,11 @@ class ExtensionService
   // pages; and perform other extension install tasks before calling
   // AddExtension.
   // |install_flags| is a bitmask of extensions::InstallFlags.
-  void AddNewOrUpdatedExtension(
-      const extensions::Extension* extension,
-      extensions::Extension::State initial_state,
-      int install_flags,
-      const syncer::StringOrdinal& page_ordinal,
-      const std::string& install_parameter,
-      const base::Optional<int>& dnr_ruleset_checksum);
+  void AddNewOrUpdatedExtension(const extensions::Extension* extension,
+                                extensions::Extension::State initial_state,
+                                int install_flags,
+                                const syncer::StringOrdinal& page_ordinal,
+                                const std::string& install_parameter);
 
   // Common helper to finish installing the given extension.
   void FinishInstallation(const extensions::Extension* extension);

diff --git a/chrome/browser/extensions/startup_helper.cc b/chrome/browser/extensions/startup_helper.cc
@@ -98,13 +98,11 @@ class ValidateCrxHelper : public SandboxedUnpackerClient {
  protected:
   ~ValidateCrxHelper() override {}
 
-  void OnUnpackSuccess(
-      const base::FilePath& temp_dir,
-      const base::FilePath& extension_root,
-      std::unique_ptr<base::DictionaryValue> original_manifest,
-      const Extension* extension,
-      const SkBitmap& install_icon,
-      const base::Optional<int>& dnr_ruleset_checksum) override {
+  void OnUnpackSuccess(const base::FilePath& temp_dir,
+                       const base::FilePath& extension_root,
+                       std::unique_ptr<base::DictionaryValue> original_manifest,
+                       const Extension* extension,
+                       const SkBitmap& install_icon) override {
     DCHECK(GetExtensionFileTaskRunner()->RunsTasksInCurrentSequence());
     success_ = true;
     BrowserThread::PostTask(

diff --git a/chrome/browser/extensions/unpacked_installer.cc b/chrome/browser/extensions/unpacked_installer.cc
@@ -354,16 +354,10 @@ bool UnpackedInstaller::IndexAndPersistRulesIfNeeded(std::string* error) {
   }
 
   std::vector<InstallWarning> warnings;
-  int ruleset_checksum;
-  if (!declarative_net_request::IndexAndPersistRules(
-          *base::ListValue::From(std::move(root)), *extension(), error,
-          &warnings, &ruleset_checksum)) {
-    return false;
-  }
-
-  dnr_ruleset_checksum_ = ruleset_checksum;
+  const bool success = declarative_net_request::IndexAndPersistRules(
+      *base::ListValue::From(std::move(root)), *extension(), error, &warnings);
   extension_->AddInstallWarnings(warnings);
-  return true;
+  return success;
 }
 
 bool UnpackedInstaller::IsLoadingUnpackedAllowed() const {
@@ -446,9 +440,8 @@ void UnpackedInstaller::InstallExtension() {
   perms_updater.InitializePermissions(extension());
   perms_updater.GrantActivePermissions(extension());
 
-  service_weak_->OnExtensionInstalled(extension(), syncer::StringOrdinal(),
-                                      kInstallFlagInstallImmediately,
-                                      dnr_ruleset_checksum_);
+  service_weak_->OnExtensionInstalled(
+      extension(), syncer::StringOrdinal(), kInstallFlagInstallImmediately);
 
   if (!callback_.is_null()) {
     callback_.Run(extension(), extension_path_, std::string());

diff --git a/chrome/browser/extensions/unpacked_installer.h b/chrome/browser/extensions/unpacked_installer.h
@@ -14,7 +14,6 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
-#include "base/optional.h"
 #include "extensions/browser/preload_check.h"
 #include "extensions/common/manifest.h"
 
@@ -168,10 +167,6 @@ class UnpackedInstaller
   // Runs the above checks.
   std::unique_ptr<PreloadCheckGroup> check_group_;
 
-  // The checksum for the indexed ruleset corresponding to the Declarative Net
-  // Request API.
-  base::Optional<int> dnr_ruleset_checksum_;
-
   CompletionCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(UnpackedInstaller);

diff --git a/chrome/browser/metrics/extensions_metrics_provider_unittest.cc b/chrome/browser/metrics/extensions_metrics_provider_unittest.cc
@@ -180,7 +180,8 @@ class ExtensionMetricsProviderInstallsTest
 TEST_F(ExtensionMetricsProviderInstallsTest, TestProtoConstruction) {
   auto add_extension = [this](const Extension* extension) {
     prefs()->OnExtensionInstalled(extension, Extension::ENABLED,
-                                  syncer::StringOrdinal(), std::string());
+                                  syncer::StringOrdinal(),
+                                  extensions::kInstallFlagNone, std::string());
   };
 
   {

diff --git a/extensions/browser/api/declarative_net_request/flat_ruleset_indexer.h b/extensions/browser/api/declarative_net_request/flat_ruleset_indexer.h
@@ -25,7 +25,7 @@ struct IndexedRule;
 class FlatRulesetIndexer {
  public:
   // Represents the address and the size of the buffer storing the ruleset.
-  using SerializedData = std::pair<const uint8_t*, size_t>;
+  using SerializedData = std::pair<uint8_t*, size_t>;
 
   FlatRulesetIndexer();
   ~FlatRulesetIndexer();