Updates BadgeManager to not use scope urls.

This is the result of some discussion at TPAC. Previously the
BadgeManager applied badges to scopes. This CL changes the BadgeManager
to work with AppIds instead (as the web facing API has been changed).

Previous CLs have changed the API exposed to the Web, so this change
only affects how Chromium stores/applies the badges.

TPAC Discussion Summary:
https://github.com/WICG/badging/issues/55

Related CLs:
https://crrev.com/c/1817999
https://crrev.com/c/1818892
https://crrev.com/c/1816002

Bug: 1006665
Change-Id: I24d18bdad5a8a0a92b0d1b5ac14052b9de8913dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1830485
Commit-Queue: Jay Harris <harrisjay@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701449}

chrome/browser/BUILD.gn

@@ -3012,7 +3012,6 @@ jumbo_split_static_library("browser") {
       "background/background_contents_service_observer.h",
       "badging/badge_manager.cc",
       "badging/badge_manager.h",
-      "badging/badge_manager_delegate.cc",
       "badging/badge_manager_delegate.h",
       "badging/badge_manager_factory.cc",
       "badging/badge_manager_factory.h",

chrome/browser/badging/badge_manager.cc

@@ -32,13 +32,9 @@ namespace badging {
 
 BadgeManager::BadgeManager(Profile* profile) {
 #if defined(OS_MACOSX)
-  SetDelegate(std::make_unique<BadgeManagerDelegateMac>(
-      profile, this,
-      &web_app::WebAppProviderBase::GetProviderBase(profile)->registrar()));
+  SetDelegate(std::make_unique<BadgeManagerDelegateMac>(profile, this));
 #elif defined(OS_WIN)
-  SetDelegate(std::make_unique<BadgeManagerDelegateWin>(
-      profile, this,
-      &web_app::WebAppProviderBase::GetProviderBase(profile)->registrar()));
+  SetDelegate(std::make_unique<BadgeManagerDelegateWin>(profile, this));
 #endif
 }
 
@@ -64,39 +60,35 @@ void BadgeManager::BindReceiver(
                                 std::move(context));
 }
 
-bool BadgeManager::HasMoreSpecificBadgeForUrl(const GURL& scope,
-                                              const GURL& url) {
-  return MostSpecificBadgeForScope(url).spec().size() > scope.spec().size();
-}
-
 base::Optional<BadgeManager::BadgeValue> BadgeManager::GetBadgeValue(
-    const GURL& scope) {
-  const GURL& most_specific = MostSpecificBadgeForScope(scope);
-  if (most_specific == GURL::EmptyGURL())
+    const web_app::AppId& app_id) {
+  const auto& it = badged_apps_.find(app_id);
+  if (it == badged_apps_.end())
     return base::nullopt;
 
-  return base::make_optional(badged_scopes_[most_specific]);
+  return it->second;
 }
 
-void BadgeManager::SetBadgeForTesting(const GURL& scope, BadgeValue value) {
-  UpdateBadge(scope, value);
+void BadgeManager::SetBadgeForTesting(const web_app::AppId& app_id,
+                                      BadgeValue value) {
+  UpdateBadge(app_id, value);
 }
 
-void BadgeManager::ClearBadgeForTesting(const GURL& scope) {
-  UpdateBadge(scope, base::nullopt);
+void BadgeManager::ClearBadgeForTesting(const web_app::AppId& app_id) {
+  UpdateBadge(app_id, base::nullopt);
 }
 
-void BadgeManager::UpdateBadge(const GURL& scope,
+void BadgeManager::UpdateBadge(const web_app::AppId& app_id,
                                base::Optional<BadgeValue> value) {
   if (!value)
-    badged_scopes_.erase(scope);
+    badged_apps_.erase(app_id);
   else
-    badged_scopes_[scope] = value.value();
+    badged_apps_[app_id] = value.value();
 
   if (!delegate_)
     return;
 
-  delegate_->OnBadgeUpdated(scope);
+  delegate_->OnAppBadgeUpdated(app_id);
 }
 
 void BadgeManager::SetBadge(blink::mojom::BadgeValuePtr mojo_value) {
@@ -107,48 +99,28 @@ void BadgeManager::SetBadge(blink::mojom::BadgeValuePtr mojo_value) {
     return;
   }
 
-  const base::Optional<GURL> app_scope =
-      GetAppScopeForContext(receivers_.current_context());
-  if (!app_scope)
+  const base::Optional<web_app::AppId> app_id =
+      GetAppIdForBadging(receivers_.current_context());
+  if (!app_id)
     return;
 
   // Convert the mojo badge representation into a BadgeManager::BadgeValue.
   BadgeValue value = mojo_value->is_flag()
                          ? base::nullopt
                          : base::make_optional(mojo_value->get_number());
-  UpdateBadge(app_scope.value(), base::make_optional(value));
+  UpdateBadge(app_id.value(), base::make_optional(value));
 }
 
 void BadgeManager::ClearBadge() {
-  const base::Optional<GURL> app_scope =
-      GetAppScopeForContext(receivers_.current_context());
-  if (!app_scope)
+  const base::Optional<web_app::AppId> app_id =
+      GetAppIdForBadging(receivers_.current_context());
+  if (!app_id)
     return;
 
-  UpdateBadge(app_scope.value(), base::nullopt);
-}
-
-GURL BadgeManager::MostSpecificBadgeForScope(const GURL& scope) {
-  const std::string& scope_string = scope.spec();
-  GURL best_match = GURL::EmptyGURL();
-  uint64_t longest_match = 0;
-
-  for (const auto& pair : badged_scopes_) {
-    const std::string& cur_scope_str = pair.first.spec();
-    if (scope_string.find(cur_scope_str) != 0)
-      continue;
-
-    if (longest_match >= cur_scope_str.size())
-      continue;
-
-    longest_match = cur_scope_str.size();
-    best_match = pair.first;
-  }
-
-  return best_match;
+  UpdateBadge(app_id.value(), base::nullopt);
 }
 
-base::Optional<GURL> BadgeManager::GetAppScopeForContext(
+base::Optional<web_app::AppId> BadgeManager::GetAppIdForBadging(
     const BindingContext& context) {
   content::RenderFrameHost* frame =
       content::RenderFrameHost::FromID(context.process_id, context.frame_id);
@@ -167,10 +139,7 @@ base::Optional<GURL> BadgeManager::GetAppScopeForContext(
 
   const base::Optional<web_app::AppId> app_id =
       registrar.FindAppWithUrlInScope(frame->GetLastCommittedURL());
-  if (!app_id)
-    return base::nullopt;
-
-  return registrar.GetAppScope(app_id.value());
+  return app_id;
 }
 
 std::string GetBadgeString(base::Optional<uint64_t> badge_content) {

chrome/browser/badging/badge_manager.h

@@ -11,6 +11,7 @@
 
 #include "base/macros.h"
 #include "base/optional.h"
+#include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "third_party/blink/public/mojom/badging/badging.mojom.h"
@@ -46,16 +47,12 @@ class BadgeManager : public KeyedService, public blink::mojom::BadgeService {
       content::RenderFrameHost* frame,
       mojo::PendingReceiver<blink::mojom::BadgeService> receiver);
 
-  // Returns whether there is a more specific badge for |url| than |scope|.
-  // Note: This function does not check that there is a badge for |scope|.
-  bool HasMoreSpecificBadgeForUrl(const GURL& scope, const GURL& url);
+  // Gets the badge for |app_id|. This will be base::nullopt if the app is not
+  // badged.
+  base::Optional<BadgeValue> GetBadgeValue(const web_app::AppId& app_id);
 
-  // Gets the most specific badge applying to |scope|. This will be
-  // base::nullopt if the scope is not badged.
-  base::Optional<BadgeValue> GetBadgeValue(const GURL& scope);
-
-  void SetBadgeForTesting(const GURL& scope, BadgeValue value);
-  void ClearBadgeForTesting(const GURL& scope);
+  void SetBadgeForTesting(const web_app::AppId& app_id, BadgeValue value);
+  void ClearBadgeForTesting(const web_app::AppId& app_id);
 
  private:
   // The BindingContext of a mojo request. Allows mojo calls to be tied back to
@@ -68,9 +65,10 @@ class BadgeManager : public KeyedService, public blink::mojom::BadgeService {
     int frame_id;
   };
 
-  // Updates the badge for |scope| to be |value|, if it is not base::nullopt.
+  // Updates the badge for |app_id| to be |value|, if it is not base::nullopt.
   // If value is |base::nullopt| then this clears the badge.
-  void UpdateBadge(const GURL& scope, base::Optional<BadgeValue> value);
+  void UpdateBadge(const web_app::AppId& app_id,
+                   base::Optional<BadgeValue> value);
 
   // blink::mojom::BadgeService:
   // Note: These are private to stop them being called outside of mojo as they
@@ -78,13 +76,10 @@ class BadgeManager : public KeyedService, public blink::mojom::BadgeService {
   void SetBadge(blink::mojom::BadgeValuePtr value) override;
   void ClearBadge() override;
 
-  // Finds the scope URL of the most specific badge for |scope|. Returns
-  // GURL::EmptyGURL() if no match is found.
-  GURL MostSpecificBadgeForScope(const GURL& scope);
-
-  // Finds the most specific app scope containing |context|. base::nullopt if
-  // no app contains |context|.
-  base::Optional<GURL> GetAppScopeForContext(const BindingContext& context);
+  // Gets the app id to badge, based on the |context|. base::nullopt if
+  // |context| isn't inside an app.
+  base::Optional<web_app::AppId> GetAppIdForBadging(
+      const BindingContext& context);
 
   // All the mojo receivers for the BadgeManager. Keeps track of the
   // render_frame the binding is associated with, so as to not have to rely
@@ -95,8 +90,8 @@ class BadgeManager : public KeyedService, public blink::mojom::BadgeService {
   // Note: This is currently only set on Windows and MacOS.
   std::unique_ptr<BadgeManagerDelegate> delegate_;
 
-  // Maps scope to badge contents.
-  std::map<GURL, BadgeValue> badged_scopes_;
+  // Maps app_id to badge contents.
+  std::map<web_app::AppId, BadgeValue> badged_apps_;
 
   DISALLOW_COPY_AND_ASSIGN(BadgeManager);
 };

chrome/browser/badging/badge_manager_delegate.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 "chrome/browser/badging/badge_manager_delegate.h"
-
-#include <vector>
-
-#include "chrome/browser/web_applications/components/app_registrar.h"
-
-namespace badging {
-
-void BadgeManagerDelegate::OnBadgeUpdated(const GURL& scope) {
-  const std::vector<web_app::AppId>& app_ids =
-      registrar()->FindAppsInScope(scope);
-
-  for (const auto& app_id : app_ids) {
-    const auto& app_scope = registrar()->GetAppScope(app_id);
-    if (!app_scope)
-      continue;
-
-    // If it wasn't the most specific badge for the app that changed, there's no
-    // need to update it.
-    if (badge_manager()->HasMoreSpecificBadgeForUrl(scope, app_scope.value()))
-      continue;
-
-    OnAppBadgeUpdated(app_id);
-  }
-}
-
-base::Optional<BadgeManager::BadgeValue> BadgeManagerDelegate::GetAppBadgeValue(
-    const web_app::AppId& app_id) {
-  const auto& scope = registrar()->GetAppScope(app_id);
-  if (!scope)
-    return base::nullopt;
-
-  return badge_manager()->GetBadgeValue(scope.value());
-}
-
-}  // namespace badging

chrome/browser/badging/badge_manager_delegate.h

@@ -12,47 +12,29 @@
 
 class Profile;
 
-namespace web_app {
-class AppRegistrar;
-}
-
 namespace badging {
 
 // A BadgeManagerDelegate is responsible for updating the UI in response to a
 // badge change.
 class BadgeManagerDelegate {
  public:
-  explicit BadgeManagerDelegate(Profile* profile,
-                                BadgeManager* badge_manager,
-                                web_app::AppRegistrar* registrar)
-      : profile_(profile),
-        badge_manager_(badge_manager),
-        registrar_(registrar) {}
+  explicit BadgeManagerDelegate(Profile* profile, BadgeManager* badge_manager)
+      : profile_(profile), badge_manager_(badge_manager) {}
 
   virtual ~BadgeManagerDelegate() = default;
 
-  // Called when the badge for |scope| has changed.
-  virtual void OnBadgeUpdated(const GURL& scope);
-
- protected:
   // Called when the badge for |app_id| has changed.
   virtual void OnAppBadgeUpdated(const web_app::AppId& app_id) = 0;
 
-  // Gets the badge for |app_id|. base::nullopt if the |app_id| is not badged.
-  base::Optional<BadgeManager::BadgeValue> GetAppBadgeValue(
-      const web_app::AppId& app_id);
-
+ protected:
   Profile* profile() { return profile_; }
   BadgeManager* badge_manager() { return badge_manager_; }
-  web_app::AppRegistrar* registrar() { return registrar_; }
 
  private:
   // The profile the badge manager delegate is associated with.
   Profile* profile_;
   // The badge manager that owns this delegate.
   BadgeManager* badge_manager_;
-  // The registrar of apps this delegate is concerned with.
-  web_app::AppRegistrar* registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(BadgeManagerDelegate);
 };

chrome/browser/badging/badge_manager_delegate_mac.cc

@@ -9,20 +9,17 @@
 #include "chrome/browser/badging/badge_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser_list.h"
-#include "chrome/browser/web_applications/components/app_registrar.h"
 #include "chrome/common/mac/app_shim.mojom.h"
 
 namespace badging {
 
-BadgeManagerDelegateMac::BadgeManagerDelegateMac(
-    Profile* profile,
-    BadgeManager* badge_manager,
-    web_app::AppRegistrar* registrar)
-    : BadgeManagerDelegate(profile, badge_manager, registrar) {}
+BadgeManagerDelegateMac::BadgeManagerDelegateMac(Profile* profile,
+                                                 BadgeManager* badge_manager)
+    : BadgeManagerDelegate(profile, badge_manager) {}
 
 void BadgeManagerDelegateMac::OnAppBadgeUpdated(const web_app::AppId& app_id) {
   const base::Optional<BadgeManager::BadgeValue>& badge =
-      GetAppBadgeValue(app_id);
+      badge_manager()->GetBadgeValue(app_id);
   SetAppBadgeLabel(app_id, badge ? badging::GetBadgeString(badge.value()) : "");
 }
 

chrome/browser/badging/badge_manager_delegate_mac.h

@@ -12,10 +12,6 @@
 
 class Profile;
 
-namespace web_app {
-class AppRegistrar;
-}
-
 namespace badging {
 
 class BadgeManager;
@@ -24,8 +20,7 @@ class BadgeManager;
 class BadgeManagerDelegateMac : public BadgeManagerDelegate {
  public:
   explicit BadgeManagerDelegateMac(Profile* profile,
-                                   BadgeManager* badge_manager,
-                                   web_app::AppRegistrar* registrar);
+                                   BadgeManager* badge_manager);
 
   void OnAppBadgeUpdated(const web_app::AppId& app_id) override;
 

chrome/browser/badging/badge_manager_delegate_unittest.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 <memory>
-#include <utility>
-
-#include "base/optional.h"
-#include "chrome/browser/badging/badge_manager.h"
-#include "chrome/browser/badging/badge_manager_factory.h"
-#include "chrome/browser/badging/test_badge_manager_delegate.h"
-#include "chrome/browser/web_applications/components/web_app_helpers.h"
-#include "chrome/browser/web_applications/test/test_app_registrar.h"
-#include "chrome/test/base/testing_profile.h"
-#include "content/public/test/browser_task_environment.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace badging {
-
-class BadgeManagerDelegateUnittest : public ::testing::Test {
- public:
-  BadgeManagerDelegateUnittest() = default;
-  ~BadgeManagerDelegateUnittest() override = default;
-
-  void SetUp() override {
-    profile_.reset(new TestingProfile());
-    registrar_.reset(new web_app::TestAppRegistrar());
-
-    badge_manager_ =
-        BadgeManagerFactory::GetInstance()->GetForProfile(profile_.get());
-
-    // Delegate lifetime is managed by BadgeManager.
-    auto owned_delegate = std::make_unique<TestBadgeManagerDelegate>(
-        profile_.get(), badge_manager_, registrar());
-    delegate_ = owned_delegate.get();
-    badge_manager_->SetDelegate(std::move(owned_delegate));
-  }
-
-  void TearDown() override { profile_.reset(); }
-
-  TestBadgeManagerDelegate* delegate() { return delegate_; }
-
-  web_app::TestAppRegistrar* registrar() const { return registrar_.get(); }
-  BadgeManager* badge_manager() const { return badge_manager_; }
-
- private:
-  std::unique_ptr<web_app::TestAppRegistrar> registrar_;
-  TestBadgeManagerDelegate* delegate_;
-  BadgeManager* badge_manager_;
-  std::unique_ptr<TestingProfile> profile_;
-  content::BrowserTaskEnvironment task_environment_;
-
-  DISALLOW_COPY_AND_ASSIGN(BadgeManagerDelegateUnittest);
-};
-
-TEST_F(BadgeManagerDelegateUnittest, InScopeAppsAreBadged) {
-  const web_app::AppId& kApp1 = "app1";
-  const web_app::AppId& kApp2 = "app2";
-
-  registrar()->AddExternalApp(kApp1, {GURL("https://example.com/app1")});
-  registrar()->AddExternalApp(kApp2, {GURL("https://example.com/app2")});
-
-  badge_manager()->SetBadgeForTesting(GURL("https://example.com/"),
-                                      base::nullopt);
-
-  EXPECT_EQ(2UL, delegate()->set_app_badges().size());
-
-  EXPECT_EQ(kApp1, delegate()->set_app_badges()[0].first);
-  EXPECT_EQ(base::nullopt, delegate()->set_app_badges()[0].second);
-
-  EXPECT_EQ(kApp2, delegate()->set_app_badges()[1].first);
-  EXPECT_EQ(base::nullopt, delegate()->set_app_badges()[1].second);
-}
-
-TEST_F(BadgeManagerDelegateUnittest, InScopeAppsAreCleared) {
-  const web_app::AppId& kApp1 = "app1";
-  const web_app::AppId& kApp2 = "app2";
-
-  registrar()->AddExternalApp(kApp1, {GURL("https://example.com/app1")});
-  registrar()->AddExternalApp(kApp2, {GURL("https://example.com/app2")});
-
-  badge_manager()->SetBadgeForTesting(GURL("https://example.com/"),
-                                      base::nullopt);
-  badge_manager()->ClearBadgeForTesting(GURL("https://example.com/"));
-
-  EXPECT_EQ(2UL, delegate()->cleared_app_badges().size());
-
-  EXPECT_EQ(kApp1, delegate()->cleared_app_badges()[0]);
-  EXPECT_EQ(kApp2, delegate()->cleared_app_badges()[1]);
-}
-
-}  // namespace badging

chrome/browser/badging/badge_manager_delegate_win.cc

@@ -12,7 +12,6 @@
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/web_applications/app_browser_controller.h"
-#include "chrome/browser/web_applications/components/app_registrar.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/strings/grit/ui_strings.h"
 
@@ -62,14 +61,13 @@ base::Optional<std::pair<std::string, std::string>> GetBadgeContentAndAlt(
 
 }  // namespace
 
-BadgeManagerDelegateWin::BadgeManagerDelegateWin(
-    Profile* profile,
-    BadgeManager* badge_manager,
-    web_app::AppRegistrar* registrar)
-    : BadgeManagerDelegate(profile, badge_manager, registrar) {}
+BadgeManagerDelegateWin::BadgeManagerDelegateWin(Profile* profile,
+                                                 BadgeManager* badge_manager)
+    : BadgeManagerDelegate(profile, badge_manager) {}
 
 void BadgeManagerDelegateWin::OnAppBadgeUpdated(const web_app::AppId& app_id) {
-  const auto& content_and_alt = GetBadgeContentAndAlt(GetAppBadgeValue(app_id));
+  const auto& content_and_alt =
+      GetBadgeContentAndAlt(badge_manager()->GetBadgeValue(app_id));
 
   for (Browser* browser : *BrowserList::GetInstance()) {
     if (!IsAppBrowser(browser, app_id))

chrome/browser/badging/badge_manager_delegate_win.h

@@ -13,10 +13,6 @@
 
 class Profile;
 
-namespace web_app {
-class AppRegistrar;
-}
-
 namespace badging {
 
 class BadgeManager;
@@ -25,8 +21,7 @@ class BadgeManager;
 class BadgeManagerDelegateWin : public BadgeManagerDelegate {
  public:
   explicit BadgeManagerDelegateWin(Profile* profile,
-                                   BadgeManager* badge_manager,
-                                   web_app::AppRegistrar* registrar);
+                                   BadgeManager* badge_manager);
 
   void OnAppBadgeUpdated(const web_app::AppId& app_id) override;
 

chrome/browser/badging/badge_manager_unittest.cc

@@ -31,7 +31,7 @@ namespace {
 typedef std::pair<GURL, base::Optional<int>> SetBadgeAction;
 
 constexpr uint64_t kBadgeContents = 1;
-const GURL kAppScope("https://example.com/app");
+const web_app::AppId kAppId = "1";
 
 }  // namespace
 
@@ -44,14 +44,13 @@ class BadgeManagerUnittest : public ::testing::Test {
 
   void SetUp() override {
     profile_.reset(new TestingProfile());
-    registrar_.reset(new web_app::TestAppRegistrar());
 
     badge_manager_ =
         BadgeManagerFactory::GetInstance()->GetForProfile(profile_.get());
 
     // Delegate lifetime is managed by BadgeManager
     auto owned_delegate = std::make_unique<TestBadgeManagerDelegate>(
-        profile_.get(), badge_manager_, registrar_.get());
+        profile_.get(), badge_manager_);
     delegate_ = owned_delegate.get();
     badge_manager_->SetDelegate(std::move(owned_delegate));
   }
@@ -63,7 +62,6 @@ class BadgeManagerUnittest : public ::testing::Test {
   BadgeManager* badge_manager() const { return badge_manager_; }
 
  private:
-  std::unique_ptr<web_app::TestAppRegistrar> registrar_;
   TestBadgeManagerDelegate* delegate_;
   BadgeManager* badge_manager_;
   content::BrowserTaskEnvironment task_environment_;
@@ -73,63 +71,63 @@ class BadgeManagerUnittest : public ::testing::Test {
 };
 
 TEST_F(BadgeManagerUnittest, SetFlagBadgeForApp) {
-  badge_manager()->SetBadgeForTesting(kAppScope, base::nullopt);
+  badge_manager()->SetBadgeForTesting(kAppId, base::nullopt);
 
-  EXPECT_EQ(1UL, delegate()->set_scope_badges().size());
-  EXPECT_EQ(kAppScope, delegate()->set_scope_badges().front().first);
-  EXPECT_EQ(base::nullopt, delegate()->set_scope_badges().front().second);
+  EXPECT_EQ(1UL, delegate()->set_badges().size());
+  EXPECT_EQ(kAppId, delegate()->set_badges().front().first);
+  EXPECT_EQ(base::nullopt, delegate()->set_badges().front().second);
 }
 
 TEST_F(BadgeManagerUnittest, SetBadgeForApp) {
-  badge_manager()->SetBadgeForTesting(kAppScope,
+  badge_manager()->SetBadgeForTesting(kAppId,
                                       base::make_optional(kBadgeContents));
 
-  EXPECT_EQ(1UL, delegate()->set_scope_badges().size());
-  EXPECT_EQ(kAppScope, delegate()->set_scope_badges().front().first);
-  EXPECT_EQ(kBadgeContents, delegate()->set_scope_badges().front().second);
+  EXPECT_EQ(1UL, delegate()->set_badges().size());
+  EXPECT_EQ(kAppId, delegate()->set_badges().front().first);
+  EXPECT_EQ(kBadgeContents, delegate()->set_badges().front().second);
 }
 
 TEST_F(BadgeManagerUnittest, SetBadgeForMultipleApps) {
-  const GURL kOtherScope("http://other.app/other");
+  const web_app::AppId kOtherAppId = "2";
   constexpr uint64_t kOtherContents = 2;
 
-  badge_manager()->SetBadgeForTesting(kAppScope,
+  badge_manager()->SetBadgeForTesting(kAppId,
                                       base::make_optional(kBadgeContents));
-  badge_manager()->SetBadgeForTesting(kOtherScope,
+  badge_manager()->SetBadgeForTesting(kOtherAppId,
                                       base::make_optional(kOtherContents));
 
-  EXPECT_EQ(2UL, delegate()->set_scope_badges().size());
+  EXPECT_EQ(2UL, delegate()->set_badges().size());
 
-  EXPECT_EQ(kAppScope, delegate()->set_scope_badges()[0].first);
-  EXPECT_EQ(kBadgeContents, delegate()->set_scope_badges()[0].second);
+  EXPECT_EQ(kAppId, delegate()->set_badges()[0].first);
+  EXPECT_EQ(kBadgeContents, delegate()->set_badges()[0].second);
 
-  EXPECT_EQ(kOtherScope, delegate()->set_scope_badges()[1].first);
-  EXPECT_EQ(kOtherContents, delegate()->set_scope_badges()[1].second);
+  EXPECT_EQ(kOtherAppId, delegate()->set_badges()[1].first);
+  EXPECT_EQ(kOtherContents, delegate()->set_badges()[1].second);
 }
 
 TEST_F(BadgeManagerUnittest, SetBadgeForAppAfterClear) {
-  badge_manager()->SetBadgeForTesting(kAppScope,
+  badge_manager()->SetBadgeForTesting(kAppId,
                                       base::make_optional(kBadgeContents));
-  badge_manager()->ClearBadgeForTesting(kAppScope);
-  badge_manager()->SetBadgeForTesting(kAppScope,
+  badge_manager()->ClearBadgeForTesting(kAppId);
+  badge_manager()->SetBadgeForTesting(kAppId,
                                       base::make_optional(kBadgeContents));
 
-  EXPECT_EQ(2UL, delegate()->set_scope_badges().size());
+  EXPECT_EQ(2UL, delegate()->set_badges().size());
 
-  EXPECT_EQ(kAppScope, delegate()->set_scope_badges()[0].first);
-  EXPECT_EQ(kBadgeContents, delegate()->set_scope_badges()[0].second);
+  EXPECT_EQ(kAppId, delegate()->set_badges()[0].first);
+  EXPECT_EQ(kBadgeContents, delegate()->set_badges()[0].second);
 
-  EXPECT_EQ(kAppScope, delegate()->set_scope_badges()[1].first);
-  EXPECT_EQ(kBadgeContents, delegate()->set_scope_badges()[1].second);
+  EXPECT_EQ(kAppId, delegate()->set_badges()[1].first);
+  EXPECT_EQ(kBadgeContents, delegate()->set_badges()[1].second);
 }
 
 TEST_F(BadgeManagerUnittest, ClearBadgeForBadgedApp) {
-  badge_manager()->SetBadgeForTesting(kAppScope,
+  badge_manager()->SetBadgeForTesting(kAppId,
                                       base::make_optional(kBadgeContents));
-  badge_manager()->ClearBadgeForTesting(kAppScope);
+  badge_manager()->ClearBadgeForTesting(kAppId);
 
-  EXPECT_EQ(1UL, delegate()->cleared_scope_badges().size());
-  EXPECT_EQ(kAppScope, delegate()->cleared_scope_badges().front());
+  EXPECT_EQ(1UL, delegate()->cleared_badges().size());
+  EXPECT_EQ(kAppId, delegate()->cleared_badges().front());
 }
 
 TEST_F(BadgeManagerUnittest, BadgingMultipleProfiles) {
@@ -137,28 +135,27 @@ TEST_F(BadgeManagerUnittest, BadgingMultipleProfiles) {
   auto* other_badge_manager =
       BadgeManagerFactory::GetInstance()->GetForProfile(other_profile.get());
 
-  web_app::TestAppRegistrar other_registrar;
   auto owned_other_delegate = std::make_unique<TestBadgeManagerDelegate>(
-      other_profile.get(), other_badge_manager, &other_registrar);
+      other_profile.get(), other_badge_manager);
   auto* other_delegate = owned_other_delegate.get();
   other_badge_manager->SetDelegate(std::move(owned_other_delegate));
 
-  other_badge_manager->SetBadgeForTesting(kAppScope, base::nullopt);
-  other_badge_manager->SetBadgeForTesting(kAppScope,
+  other_badge_manager->SetBadgeForTesting(kAppId, base::nullopt);
+  other_badge_manager->SetBadgeForTesting(kAppId,
                                           base::make_optional(kBadgeContents));
-  other_badge_manager->SetBadgeForTesting(kAppScope, base::nullopt);
-  other_badge_manager->ClearBadgeForTesting(kAppScope);
+  other_badge_manager->SetBadgeForTesting(kAppId, base::nullopt);
+  other_badge_manager->ClearBadgeForTesting(kAppId);
 
-  badge_manager()->ClearBadgeForTesting(kAppScope);
+  badge_manager()->ClearBadgeForTesting(kAppId);
 
-  EXPECT_EQ(3UL, other_delegate->set_scope_badges().size());
-  EXPECT_EQ(0UL, delegate()->set_scope_badges().size());
+  EXPECT_EQ(3UL, other_delegate->set_badges().size());
+  EXPECT_EQ(0UL, delegate()->set_badges().size());
 
-  EXPECT_EQ(1UL, other_delegate->cleared_scope_badges().size());
-  EXPECT_EQ(1UL, delegate()->cleared_scope_badges().size());
+  EXPECT_EQ(1UL, other_delegate->cleared_badges().size());
+  EXPECT_EQ(1UL, delegate()->cleared_badges().size());
 
-  EXPECT_EQ(kAppScope, other_delegate->set_scope_badges().back().first);
-  EXPECT_EQ(base::nullopt, other_delegate->set_scope_badges().back().second);
+  EXPECT_EQ(kAppId, other_delegate->set_badges().back().first);
+  EXPECT_EQ(base::nullopt, other_delegate->set_badges().back().second);
 }
 
 // Tests methods which call into the badge manager delegate do not crash when
@@ -166,46 +163,10 @@ TEST_F(BadgeManagerUnittest, BadgingMultipleProfiles) {
 TEST_F(BadgeManagerUnittest, BadgingWithNoDelegateDoesNotCrash) {
   badge_manager()->SetDelegate(nullptr);
 
-  badge_manager()->SetBadgeForTesting(kAppScope, base::nullopt);
-  badge_manager()->SetBadgeForTesting(kAppScope,
+  badge_manager()->SetBadgeForTesting(kAppId, base::nullopt);
+  badge_manager()->SetBadgeForTesting(kAppId,
                                       base::make_optional(kBadgeContents));
-  badge_manager()->SetBadgeForTesting(GURL(), base::nullopt);
-  badge_manager()->SetBadgeForTesting(GURL(),
-                                      base::make_optional(kBadgeContents));
-  badge_manager()->ClearBadgeForTesting(GURL());
-}
-
-TEST_F(BadgeManagerUnittest, MostSpecificBadgeApplies) {
-  const GURL origin("https://example.com/");
-  const GURL app("https://example.com/app");
-  const GURL app_page("https://example.com/app/page/1");
-
-  const base::Optional<BadgeManager::BadgeValue> origin_badge =
-      base::make_optional(base::Optional<uint64_t>(1U));
-  const base::Optional<BadgeManager::BadgeValue> app_badge =
-      base::make_optional(base::Optional<uint64_t>(1U));
-  const base::Optional<BadgeManager::BadgeValue> app_page_badge =
-      base::make_optional(base::Optional<uint64_t>(1U));
-
-  badge_manager()->SetBadgeForTesting(origin, origin_badge.value());
-  EXPECT_EQ(badge_manager()->GetBadgeValue(origin), origin_badge);
-  EXPECT_EQ(badge_manager()->GetBadgeValue(app), origin_badge);
-  EXPECT_EQ(badge_manager()->GetBadgeValue(app_page), origin_badge);
-
-  badge_manager()->SetBadgeForTesting(app, app_badge.value());
-  EXPECT_EQ(badge_manager()->GetBadgeValue(origin), origin_badge);
-  EXPECT_EQ(badge_manager()->GetBadgeValue(app), app_badge);
-  EXPECT_EQ(badge_manager()->GetBadgeValue(app_page), app_badge);
-
-  badge_manager()->SetBadgeForTesting(app_page, app_page_badge.value());
-  EXPECT_EQ(badge_manager()->GetBadgeValue(origin), origin_badge);
-  EXPECT_EQ(badge_manager()->GetBadgeValue(app), app_badge);
-  EXPECT_EQ(badge_manager()->GetBadgeValue(app_page), app_page_badge);
-
-  badge_manager()->ClearBadgeForTesting(app);
-  EXPECT_EQ(badge_manager()->GetBadgeValue(origin), origin_badge);
-  EXPECT_EQ(badge_manager()->GetBadgeValue(app), origin_badge);
-  EXPECT_EQ(badge_manager()->GetBadgeValue(app_page), app_page_badge);
+  badge_manager()->ClearBadgeForTesting(kAppId);
 }
 
 }  // namespace badging

chrome/browser/badging/test_badge_manager_delegate.cc

@@ -11,11 +11,9 @@
 
 namespace badging {
 
-TestBadgeManagerDelegate::TestBadgeManagerDelegate(
-    Profile* profile,
-    BadgeManager* badge_manager,
-    web_app::AppRegistrar* registrar)
-    : BadgeManagerDelegate(profile, badge_manager, registrar) {}
+TestBadgeManagerDelegate::TestBadgeManagerDelegate(Profile* profile,
+                                                   BadgeManager* badge_manager)
+    : BadgeManagerDelegate(profile, badge_manager) {}
 
 TestBadgeManagerDelegate::~TestBadgeManagerDelegate() = default;
 
@@ -24,32 +22,20 @@ void TestBadgeManagerDelegate::SetOnBadgeChanged(
   on_badge_changed_ = on_badge_changed;
 }
 
-void TestBadgeManagerDelegate::OnBadgeUpdated(const GURL& scope) {
-  BadgeManagerDelegate::OnBadgeUpdated(scope);
-
-  const auto& value = badge_manager()->GetBadgeValue(scope);
+void TestBadgeManagerDelegate::OnAppBadgeUpdated(const web_app::AppId& app_id) {
+  const auto& value = badge_manager()->GetBadgeValue(app_id);
   if (!value)
-    cleared_scope_badges_.push_back(scope);
+    cleared_badges_.push_back(app_id);
   else
-    set_scope_badges_.push_back(std::make_pair(scope, value.value()));
+    set_badges_.push_back(std::make_pair(app_id, value.value()));
 
   if (on_badge_changed_)
     on_badge_changed_.Run();
 }
 
-void TestBadgeManagerDelegate::OnAppBadgeUpdated(const web_app::AppId& app_id) {
-  const auto& value = GetAppBadgeValue(app_id);
-  if (!value)
-    cleared_app_badges_.push_back(app_id);
-  else
-    set_app_badges_.push_back(std::make_pair(app_id, value.value()));
-}
-
 void TestBadgeManagerDelegate::ResetBadges() {
-  cleared_app_badges_.clear();
-  set_app_badges_.clear();
-  cleared_scope_badges_.clear();
-  set_scope_badges_.clear();
+  cleared_badges_.clear();
+  set_badges_.clear();
 }
 
 }  // namespace badging

chrome/browser/badging/test_badge_manager_delegate.h

@@ -15,10 +15,6 @@
 
 class Profile;
 
-namespace web_app {
-class AppRegistrar;
-}
-
 namespace badging {
 
 class BadgeManager;
@@ -29,9 +25,7 @@ using ScopeBadge = std::pair<GURL, BadgeManager::BadgeValue>;
 // Testing delegate that records badge changes for apps.
 class TestBadgeManagerDelegate : public BadgeManagerDelegate {
  public:
-  TestBadgeManagerDelegate(Profile* profile,
-                           BadgeManager* badge_manager,
-                           web_app::AppRegistrar* registrar);
+  TestBadgeManagerDelegate(Profile* profile, BadgeManager* badge_manager);
   ~TestBadgeManagerDelegate() override;
 
   // Sets a callback to fire when a badge is set or cleared.
@@ -40,27 +34,19 @@ class TestBadgeManagerDelegate : public BadgeManagerDelegate {
   // Resets the lists of cleared and set badges.
   void ResetBadges();
 
-  std::vector<web_app::AppId> cleared_app_badges() {
-    return cleared_app_badges_;
-  }
-  std::vector<AppBadge> set_app_badges() { return set_app_badges_; }
-  std::vector<GURL> cleared_scope_badges() { return cleared_scope_badges_; }
-  std::vector<ScopeBadge> set_scope_badges() { return set_scope_badges_; }
+  std::vector<web_app::AppId> cleared_badges() { return cleared_badges_; }
+  std::vector<AppBadge> set_badges() { return set_badges_; }
 
   // BadgeManagerDelegate:
-  void OnBadgeUpdated(const GURL& scope) override;
+  void OnAppBadgeUpdated(const web_app::AppId& app_badge) override;
 
  protected:
-  // BadgeManagerDelegate:
-  void OnAppBadgeUpdated(const web_app::AppId& app_badge) override;
 
  private:
   base::RepeatingCallback<void()> on_badge_changed_;
 
-  std::vector<web_app::AppId> cleared_app_badges_;
-  std::vector<AppBadge> set_app_badges_;
-  std::vector<GURL> cleared_scope_badges_;
-  std::vector<ScopeBadge> set_scope_badges_;
+  std::vector<web_app::AppId> cleared_badges_;
+  std::vector<AppBadge> set_badges_;
 };
 
 }  // namespace badging

chrome/browser/ui/web_applications/web_app_badging_browsertest.cc

@@ -72,9 +72,8 @@ class WebAppBadgingBrowserTest : public WebAppControllerBrowserTest {
     // The delegate is owned by the badge manager. We hold a pointer to it for
     // the test.
     std::unique_ptr<badging::TestBadgeManagerDelegate> owned_delegate =
-        std::make_unique<badging::TestBadgeManagerDelegate>(
-            profile(), badge_manager,
-            &WebAppProviderBase::GetProviderBase(profile())->registrar());
+        std::make_unique<badging::TestBadgeManagerDelegate>(profile(),
+                                                            badge_manager);
     owned_delegate->SetOnBadgeChanged(base::BindRepeating(
         &WebAppBadgingBrowserTest::OnBadgeChanged, base::Unretained(this)));
     delegate_ = owned_delegate.get();
@@ -85,18 +84,18 @@ class WebAppBadgingBrowserTest : public WebAppControllerBrowserTest {
   void OnBadgeChanged() {
     // This is only set up to deal with one badge change at a time, in order to
     // make asserting the result of a badge change easier.
-    int total_changes = delegate_->cleared_app_badges().size() +
-                        delegate_->set_app_badges().size();
+    int total_changes =
+        delegate_->cleared_badges().size() + delegate_->set_badges().size();
     ASSERT_EQ(total_changes, 1);
 
-    if (delegate_->cleared_app_badges().size()) {
-      changed_app_id_ = delegate_->cleared_app_badges()[0];
+    if (delegate_->cleared_badges().size()) {
+      changed_app_id_ = delegate_->cleared_badges()[0];
       was_cleared_ = true;
     }
 
-    if (delegate_->set_app_badges().size() == 1) {
-      changed_app_id_ = delegate_->set_app_badges()[0].first;
-      last_badge_content_ = delegate_->set_app_badges()[0].second;
+    if (delegate_->set_badges().size() == 1) {
+      changed_app_id_ = delegate_->set_badges()[0].first;
+      last_badge_content_ = delegate_->set_badges()[0].second;
       was_flagged_ = last_badge_content_ == base::nullopt;
     }
 

chrome/test/BUILD.gn

@@ -3666,7 +3666,6 @@ test("unit_tests") {
     # !is_android
     sources += [
       # Badging isn't supported on Android.
-      "../browser/badging/badge_manager_delegate_unittest.cc",
       "../browser/badging/badge_manager_unittest.cc",
       "../browser/badging/test_badge_manager_delegate.cc",
       "../browser/badging/test_badge_manager_delegate.h",