[DNR] Return placeholder for getBadgeText if badge displays action count

If an extension has enabled displaying matched action count as its badge
text and the badge text for a tab is not being overridden with one set
by setBadgeText, then calling getBadgeText should return a placeholder
value.

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

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

@@ -2713,11 +2713,12 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest,
   ASSERT_NO_FATAL_FAILURE(LoadExtensionWithRules(
       rules, "test_extension", {URLPattern::kAllUrlsPattern}));
 
+  const ExtensionId& extension_id = last_loaded_extension_id();
   const Extension* dnr_extension = extension_registry()->GetExtensionById(
-      last_loaded_extension_id(), extensions::ExtensionRegistry::ENABLED);
+      extension_id, extensions::ExtensionRegistry::ENABLED);
 
-  ExtensionPrefs::Get(profile())->SetDNRUseActionCountAsBadgeText(
-      last_loaded_extension_id(), true);
+  ExtensionPrefs::Get(profile())->SetDNRUseActionCountAsBadgeText(extension_id,
+                                                                  true);
 
   ExtensionAction* action =
       ExtensionActionManager::Get(web_contents()->GetBrowserContext())
@@ -2784,6 +2785,90 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest,
   EXPECT_EQ(first_tab_badge_text, action->GetDisplayBadgeText(first_tab_id));
 }
 
+// Test that the extension cannot retrieve the number of actions matched
+// from the badge text by calling chrome.browserAction.getBadgeText.
+IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest,
+                       GetBadgeTextForActionsMatched) {
+  auto query_badge_text_from_ext = [this](const ExtensionId& extension_id,
+                                          int tab_id) {
+    static constexpr char kBadgeTextQueryScript[] = R"(
+        chrome.browserAction.getBadgeText({tabId: %d}, badgeText => {
+          window.domAutomationController.send(badgeText);
+        });
+      )";
+
+    return ExecuteScriptInBackgroundPage(
+        extension_id, base::StringPrintf(kBadgeTextQueryScript, tab_id));
+  };
+
+  // Load the extension with a background script so scripts can be run from its
+  // generated background page.
+  set_has_background_script(true);
+
+  ASSERT_NO_FATAL_FAILURE(LoadExtensionWithRules(
+      {}, "test_extension", {URLPattern::kAllUrlsPattern}));
+
+  const ExtensionId& extension_id = last_loaded_extension_id();
+  const Extension* dnr_extension = extension_registry()->GetExtensionById(
+      extension_id, extensions::ExtensionRegistry::ENABLED);
+
+  ExtensionAction* action =
+      ExtensionActionManager::Get(web_contents()->GetBrowserContext())
+          ->GetExtensionAction(*dnr_extension);
+
+  const std::string default_badge_text = "asdf";
+  action->SetBadgeText(ExtensionAction::kDefaultTabId, default_badge_text);
+
+  const GURL page_url = embedded_test_server()->GetURL(
+      "norulesmatched.com", "/pages_with_script/index.html");
+  ui_test_utils::NavigateToURL(browser(), page_url);
+  ASSERT_TRUE(WasFrameWithScriptLoaded(GetMainFrame()));
+
+  // The preference is initially turned off. Both the visible badge text and the
+  // badge text queried by the extension using getBadgeText() should return the
+  // default badge text.
+  int first_tab_id = ExtensionTabUtil::GetTabId(web_contents());
+  EXPECT_EQ(default_badge_text, action->GetDisplayBadgeText(first_tab_id));
+
+  std::string queried_badge_text =
+      query_badge_text_from_ext(extension_id, first_tab_id);
+  EXPECT_EQ(default_badge_text, queried_badge_text);
+
+  ExtensionPrefs::Get(profile())->SetDNRUseActionCountAsBadgeText(extension_id,
+                                                                  true);
+  // TODO(crbug.com/979068): Remove the navigation once we update extension
+  // action immediately upon toggling preference.
+  ui_test_utils::NavigateToURL(browser(), page_url);
+  ASSERT_TRUE(WasFrameWithScriptLoaded(GetMainFrame()));
+
+  // Since the preference is on for the current tab, attempting to query the
+  // badge text from the extension should return the placeholder text instead of
+  // the matched action count.
+  queried_badge_text = query_badge_text_from_ext(extension_id, first_tab_id);
+  EXPECT_EQ(declarative_net_request::kActionCountPlaceholderBadgeText,
+            queried_badge_text);
+
+  // The displayed badge text should show "0" as no actions have been matched.
+  EXPECT_EQ("0", action->GetDisplayBadgeText(first_tab_id));
+
+  ExtensionPrefs::Get(profile())->SetDNRUseActionCountAsBadgeText(extension_id,
+                                                                  false);
+  // TODO(crbug.com/979068): Remove the navigation once we update extension
+  // action immediately upon toggling preference.
+  ui_test_utils::NavigateToURL(browser(), page_url);
+  ASSERT_TRUE(WasFrameWithScriptLoaded(GetMainFrame()));
+
+  // Switching the preference off should cause the extension queried badge text
+  // to be the explicitly set badge text for this tab if it exists. In this
+  // case, the queried badge text should be the default badge text.
+  queried_badge_text = query_badge_text_from_ext(extension_id, first_tab_id);
+  EXPECT_EQ(default_badge_text, queried_badge_text);
+
+  // The displayed badge text should be the default badge text now that the
+  // preference is off.
+  EXPECT_EQ(default_badge_text, action->GetDisplayBadgeText(first_tab_id));
+}
+
 // Test that the action matched badge text for an extension is visible in an
 // incognito context if the extension is incognito enabled.
 IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest,

chrome/browser/extensions/api/extension_action/extension_action_api.cc

@@ -31,6 +31,7 @@
 #include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
 #include "chrome/common/extensions/api/extension_action/action_info.h"
 #include "content/public/browser/notification_service.h"
+#include "extensions/browser/api/declarative_net_request/constants.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_host.h"
 #include "extensions/browser/extension_registry.h"
@@ -469,10 +470,19 @@ ExtensionActionGetPopupFunction::RunExtensionAction() {
 
 ExtensionFunction::ResponseAction
 ExtensionActionGetBadgeTextFunction::RunExtensionAction() {
-  // TODO(crbug.com/990224): Return a placeholder value if the extension has
-  // called setActionCountAsBadgeText(true).
-  return RespondNow(OneArgument(std::make_unique<base::Value>(
-      extension_action_->GetExplicitlySetBadgeText(tab_id_))));
+  // Return a placeholder value if the extension has called
+  // setActionCountAsBadgeText(true) and the badge count shown for this tab is
+  // the number of actions matched.
+  std::string badge_text =
+      extension_action_->UseDNRActionCountAsBadgeText(tab_id_)
+          ? declarative_net_request::kActionCountPlaceholderBadgeText
+          : extension_action_->GetExplicitlySetBadgeText(tab_id_);
+
+  // TODO(crbug.com/990224): Document this behavior once
+  // chrome.declarativeNetRequest.setActionCountAsBadgeText is promoted to beta
+  // from trunk.
+  return RespondNow(
+      OneArgument(std::make_unique<base::Value>(std::move(badge_text))));
 }
 
 ExtensionFunction::ResponseAction

chrome/browser/extensions/extension_action.cc

@@ -253,16 +253,15 @@ gfx::Image ExtensionAction::GetPlaceholderIconImage() const {
 }
 
 std::string ExtensionAction::GetDisplayBadgeText(int tab_id) const {
+  return UseDNRActionCountAsBadgeText(tab_id)
+             ? base::NumberToString(GetDNRActionCount(tab_id))
+             : GetExplicitlySetBadgeText(tab_id);
+}
+
+bool ExtensionAction::UseDNRActionCountAsBadgeText(int tab_id) const {
   // Tab specific badge text set by an extension overrides the automatically set
   // action count.
-  if (HasBadgeText(tab_id))
-    return GetExplicitlySetBadgeText(tab_id);
-  if (HasDNRActionCount(tab_id))
-    return base::NumberToString(GetDNRActionCount(tab_id));
-
-  // Return the default badge text or an empty string if there is no badge text
-  // set for this tab.
-  return GetExplicitlySetBadgeText(kDefaultTabId);
+  return !HasBadgeText(tab_id) && HasDNRActionCount(tab_id);
 }
 
 bool ExtensionAction::HasPopupUrl(int tab_id) const {

chrome/browser/extensions/extension_action.h

@@ -161,6 +161,10 @@ class ExtensionAction {
   // - The default badge text, if set, otherwise: an empty string.
   std::string GetDisplayBadgeText(int tab_id) const;
 
+  // Returns whether this extension action is using the DNR action count as its
+  // badge text.
+  bool UseDNRActionCountAsBadgeText(int tab_id) const;
+
   // Set this action's badge visibility on a specific tab.  Returns true if
   // the visibility has changed.
   bool SetIsVisible(int tab_id, bool value);

extensions/browser/api/declarative_net_request/constants.cc

@@ -76,5 +76,8 @@ const char kUpdateDynamicRulesStatusHistogram[] =
 const char kReadDynamicRulesJSONStatusHistogram[] =
     "Extensions.DeclarativeNetRequest.ReadDynamicRulesJSONStatus";
 
+const char kActionCountPlaceholderBadgeText[] =
+    "<<declarativeNetRequestActionCount>>";
+
 }  // namespace declarative_net_request
 }  // namespace extensions

extensions/browser/api/declarative_net_request/constants.h

@@ -128,6 +128,10 @@ extern const char kManifestRulesCountHistogram[];
 extern const char kUpdateDynamicRulesStatusHistogram[];
 extern const char kReadDynamicRulesJSONStatusHistogram[];
 
+// Placeholder text to use for getBadgeText extension function call, when the
+// badge text is set to the DNR action count.
+extern const char kActionCountPlaceholderBadgeText[];
+
 }  // namespace declarative_net_request
 }  // namespace extensions
 

extensions/common/api/declarative_net_request/test_utils.cc

@@ -202,6 +202,7 @@ std::unique_ptr<base::DictionaryValue> CreateManifest(
       .Set("background", DictionaryBuilder()
                              .Set("scripts", ToListValue(background_scripts))
                              .Build())
+      .Set(keys::kBrowserAction, DictionaryBuilder().Build())
       .Build();
 }