[DNR] Update allocation for unpacked reloads

Reloading an unpacked extension is functionally identical to
uninstalling it, then installing again from its unpacked directory.

Rule allocation should be updated in case the unpacked directory's rule
count changes.

Bug: 983299
Change-Id: I8c73847ff025bed62c4adbf0810b1c29878f4f44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2556759
Commit-Queue: Kelvin Jiang <kelvinjiang@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831133}

chrome/browser/extensions/api/declarative_net_request/declarative_net_request_unittest.cc

@@ -194,6 +194,8 @@ class DeclarativeNetRequestUnittest : public DNRTestBase {
     tester.ExpectTotalCount(kManifestRulesCountHistogram, 0u);
   }
 
+  // void Update
+
   enum class RulesetScope { kDynamic, kSession };
 
   // Runs the updateDynamicRules/updateSessionRules extension function based on
@@ -2184,6 +2186,48 @@ TEST_P(MultipleRulesetsGlobalRulesTest, ReclaimAllocationOnUnload) {
   CheckExtensionAllocationInPrefs(second_extension_id, ext_2_allocation);
 }
 
+using MultipleRulesetsGlobalRulesTest_Unpacked =
+    MultipleRulesetsGlobalRulesTest;
+
+// Test that reloading an unpacked extension is functionally identical to
+// uninstalling then reinstalling it for the purpose of global rule allocation,
+// and the allocation should reflect changes made to the extension.
+TEST_P(MultipleRulesetsGlobalRulesTest_Unpacked, UpdateAllocationOnReload) {
+  AddRuleset(CreateRuleset(kId1, 250, 0, true));
+
+  RulesetManagerObserver ruleset_waiter(manager());
+  LoadAndExpectSuccess(250);
+  ruleset_waiter.WaitForExtensionsWithRulesetsCount(1);
+  ExtensionId extension_id = extension()->id();
+
+  // The 150 rules that contribute to the global pool should be
+  // tracked.
+  GlobalRulesTracker& global_rules_tracker =
+      RulesMonitorService::Get(browser_context())->global_rules_tracker();
+  EXPECT_EQ(150u, global_rules_tracker.GetAllocatedGlobalRuleCountForTesting());
+
+  // An entry for these 150 rules should be persisted for the extension in
+  // prefs.
+  CheckExtensionAllocationInPrefs(extension_id, 150);
+
+  // Replace ruleset |kId1| with a smaller ruleset |kId2| and persist the
+  // ruleset to the extension's directory via WriteExtensionData().
+  ClearRulesets();
+  AddRuleset(CreateRuleset(kId2, 150, 0, true));
+  WriteExtensionData();
+
+  // Reload the extension. For unpacked extensions this is functionally
+  // equivalent to uninstalling the extension then installing it again based on
+  // the contents of the extension's directory.
+  service()->ReloadExtension(extension_id);
+  ruleset_waiter.WaitForExtensionsWithRulesetsCount(1);
+
+  // File changes to the extension's ruleset should take effect after it is
+  // reloaded.
+  EXPECT_EQ(50u, global_rules_tracker.GetAllocatedGlobalRuleCountForTesting());
+  CheckExtensionAllocationInPrefs(extension_id, 50);
+}
+
 INSTANTIATE_TEST_SUITE_P(All,
                          SingleRulesetTest,
                          ::testing::Values(ExtensionLoadType::PACKED,
@@ -2204,6 +2248,10 @@ INSTANTIATE_TEST_SUITE_P(All,
                          ::testing::Values(ExtensionLoadType::PACKED,
                                            ExtensionLoadType::UNPACKED));
 
+INSTANTIATE_TEST_SUITE_P(All,
+                         MultipleRulesetsGlobalRulesTest_Unpacked,
+                         ::testing::Values(ExtensionLoadType::UNPACKED));
+
 }  // namespace
 }  // namespace declarative_net_request
 }  // namespace extensions

extensions/browser/api/declarative_net_request/rules_monitor_service.cc

@@ -67,17 +67,23 @@ void LogLoadRulesetResult(LoadRulesetResult result) {
 
 // Returns whether the extension's allocation should be released. This would
 // return true for cases where we expect the extension to be unloaded for a
-// while.
+// while or if the extension directory's contents changed in a reload.
 bool ShouldReleaseAllocationOnUnload(const ExtensionPrefs* prefs,
-                                     const ExtensionId& extension_id,
+                                     const Extension& extension,
                                      UnloadedExtensionReason reason) {
   if (reason == UnloadedExtensionReason::DISABLE) {
     static constexpr int kReleaseAllocationDisableReasons =
         disable_reason::DISABLE_BLOCKED_BY_POLICY |
         disable_reason::DISABLE_REMOTELY_FOR_MALWARE;
 
-    return (prefs->GetDisableReasons(extension_id) &
-            kReleaseAllocationDisableReasons) != 0;
+    // Release allocation on reload of an unpacked extension and treat it as a
+    // new install since the extension directory's contents may have changed.
+    bool is_unpacked_reload =
+        Manifest::IsUnpackedLocation(extension.location()) &&
+        prefs->HasDisableReason(extension.id(), disable_reason::DISABLE_RELOAD);
+
+    return is_unpacked_reload || (prefs->GetDisableReasons(extension.id()) &
+                                  kReleaseAllocationDisableReasons) != 0;
   }
 
   return reason == UnloadedExtensionReason::BLOCKLIST;
@@ -379,8 +385,9 @@ void RulesMonitorService::OnExtensionUnloaded(
     if (reason != UnloadedExtensionReason::UPDATE)
       prefs_->SetDNRKeepExcessAllocation(extension->id(), false);
 
-    if (ShouldReleaseAllocationOnUnload(prefs_, extension->id(), reason))
+    if (ShouldReleaseAllocationOnUnload(prefs_, *extension, reason)) {
       global_rules_tracker_.ClearExtensionAllocation(extension->id());
+    }
   }
 
   // Return early if the extension does not have an active indexed ruleset.