Add a CookieManager interface for monitoring all cookie changes.

Bug: 721395 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I8a2cd64bc04c33fa984af3ebff7d053738f5ccd4 Reviewed-on: https://chromium-review.googlesource.com/806355 Reviewed-by: Helen Li <xunjieli@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: Randy Smith <rdsmith@chromium.org> Cr-Commit-Position: refs/heads/master@{#521801}
Denger-Network · Dec 5, 2017 · d227f18 · d227f18
1 parent 0fdb7a8
commit d227f18
Show file tree

Hide file tree

Showing 4 changed files with 138 additions and 0 deletions.
diff --git a/content/network/cookie_manager.cc b/content/network/cookie_manager.cc
@@ -208,6 +208,43 @@ void CookieManager::RequestNotification(
   notifications_registered_.push_back(std::move(notification_registration));
 }
 
+void CookieManager::RequestGlobalNotifications(
+    network::mojom::CookieChangeNotificationPtr notification_pointer) {
+  std::unique_ptr<NotificationRegistration> notification_registration(
+      std::make_unique<NotificationRegistration>());
+  notification_registration->notification_pointer =
+      std::move(notification_pointer);
+
+  notification_registration->subscription =
+      cookie_store_->AddCallbackForAllChanges(
+          base::Bind(&CookieManager::CookieChanged,
+                     // base::Unretained is safe as destruction of the
+                     // CookieManager will also destroy the
+                     // notifications_registered list (which this object will be
+                     // inserted into, below), which will destroy the
+                     // CookieChangedSubscription, unregistering the callback.
+                     base::Unretained(this),
+                     // base::Unretained is safe as destruction of the
+                     // NotificationRegistration will also destroy the
+                     // CookieChangedSubscription, unregistering the callback.
+                     base::Unretained(notification_registration.get())));
+
+  notification_registration->notification_pointer.set_connection_error_handler(
+      base::BindOnce(&CookieManager::NotificationPipeBroken,
+                     // base::Unretained is safe as destruction of the
+                     // CookieManager will also destroy the
+                     // notifications_registered list (which this object will be
+                     // inserted into, below), which will destroy the
+                     // notification_pointer, rendering this callback moot.
+                     base::Unretained(this),
+                     // base::Unretained is safe as destruction of the
+                     // NotificationRegistration will also destroy the
+                     // CookieChangedSubscription, unregistering the callback.
+                     base::Unretained(notification_registration.get())));
+
+  notifications_registered_.push_back(std::move(notification_registration));
+}
+
 void CookieManager::CookieChanged(NotificationRegistration* registration,
                                   const net::CanonicalCookie& cookie,
                                   net::CookieStore::ChangeCause cause) {

diff --git a/content/network/cookie_manager.h b/content/network/cookie_manager.h
@@ -54,6 +54,8 @@ class CONTENT_EXPORT CookieManager : public network::mojom::CookieManager {
                            const std::string& name,
                            network::mojom::CookieChangeNotificationPtr
                                notification_pointer) override;
+  void RequestGlobalNotifications(network::mojom::CookieChangeNotificationPtr
+                                      notification_pointer) override;
   void CloneInterface(
       network::mojom::CookieManagerRequest new_interface) override;
 

diff --git a/content/network/cookie_manager_unittest.cc b/content/network/cookie_manager_unittest.cc
@@ -1422,6 +1422,96 @@ TEST_F(CookieManagerTest, Notification) {
             notifications[0].cause);
 }
 
+TEST_F(CookieManagerTest, GlobalNotifications) {
+  const std::string kExampleHost = "www.example.com";
+  const std::string kThisHost = "www.this.com";
+  const std::string kThisETLDP1 = "this.com";
+  const std::string kThatHost = "www.that.com";
+
+  network::mojom::CookieChangeNotificationPtr ptr;
+  network::mojom::CookieChangeNotificationRequest request(
+      mojo::MakeRequest(&ptr));
+
+  CookieChangeNotification notification(std::move(request));
+
+  cookie_service_client()->RequestGlobalNotifications(std::move(ptr));
+
+  std::vector<CookieChangeNotification::Notification> notifications;
+  notification.GetCurrentNotifications(&notifications);
+  EXPECT_EQ(0u, notifications.size());
+  notifications.clear();
+
+  // Confirm the right notification is seen from setting a cookie.
+  service_wrapper()->SetCanonicalCookie(
+      net::CanonicalCookie("Thing1", "val", kExampleHost, "/", base::Time(),
+                           base::Time(), base::Time(), /*secure=*/false,
+                           /*httponly=*/false,
+                           net::CookieSameSite::NO_RESTRICTION,
+                           net::COOKIE_PRIORITY_MEDIUM),
+      true, true);
+
+  // Expect asynchrony
+  notification.GetCurrentNotifications(&notifications);
+  EXPECT_EQ(0u, notifications.size());
+  notifications.clear();
+
+  base::RunLoop().RunUntilIdle();
+  notification.GetCurrentNotifications(&notifications);
+  EXPECT_EQ(1u, notifications.size());
+  EXPECT_EQ("Thing1", notifications[0].cookie.Name());
+  EXPECT_EQ("val", notifications[0].cookie.Value());
+  EXPECT_EQ(kExampleHost, notifications[0].cookie.Domain());
+  EXPECT_EQ("/", notifications[0].cookie.Path());
+  EXPECT_EQ(network::mojom::CookieChangeCause::INSERTED,
+            notifications[0].cause);
+  notifications.clear();
+
+  // Set two cookies in a row on different domains and confirm they are both
+  // signalled.
+  service_wrapper()->SetCanonicalCookie(
+      net::CanonicalCookie("Thing1", "val", kThisHost, "/", base::Time(),
+                           base::Time(), base::Time(), /*secure=*/false,
+                           /*httponly=*/false,
+                           net::CookieSameSite::NO_RESTRICTION,
+                           net::COOKIE_PRIORITY_MEDIUM),
+      true, true);
+  service_wrapper()->SetCanonicalCookie(
+      net::CanonicalCookie("Thing2", "val", kThatHost, "/", base::Time(),
+                           base::Time(), base::Time(), /*secure=*/false,
+                           /*httponly=*/false,
+                           net::CookieSameSite::NO_RESTRICTION,
+                           net::COOKIE_PRIORITY_MEDIUM),
+      true, true);
+
+  base::RunLoop().RunUntilIdle();
+  notification.GetCurrentNotifications(&notifications);
+  EXPECT_EQ(2u, notifications.size());
+  EXPECT_EQ("Thing1", notifications[0].cookie.Name());
+  EXPECT_EQ(network::mojom::CookieChangeCause::INSERTED,
+            notifications[0].cause);
+  EXPECT_EQ("Thing2", notifications[1].cookie.Name());
+  EXPECT_EQ(network::mojom::CookieChangeCause::INSERTED,
+            notifications[1].cause);
+  notifications.clear();
+
+  // Delete cookies matching one domain; should produce one notification.
+  network::mojom::CookieDeletionFilter filter;
+  filter.including_domains = std::vector<std::string>();
+  filter.including_domains->push_back(kThisETLDP1);
+  // If this test fails, it may indicate a problem which will lead to
+  // no notifications being generated and the test hanging, so assert.
+  ASSERT_EQ(1u, service_wrapper()->DeleteCookies(filter));
+
+  // The notification may already have arrived, or it may arrive in the future.
+  notification.WaitForSomeNotification();
+  notification.GetCurrentNotifications(&notifications);
+  ASSERT_EQ(1u, notifications.size());
+  EXPECT_EQ("Thing1", notifications[0].cookie.Name());
+  EXPECT_EQ(kThisHost, notifications[0].cookie.Domain());
+  EXPECT_EQ(network::mojom::CookieChangeCause::EXPLICIT,
+            notifications[0].cause);
+}
+
 // Confirm the service operates properly if a returned notification interface
 // is destroyed.
 TEST_F(CookieManagerTest, NotificationRequestDestroyed) {

diff --git a/services/network/public/interfaces/cookie_manager.mojom b/services/network/public/interfaces/cookie_manager.mojom
@@ -167,6 +167,15 @@ interface CookieManager {
       string name,
       CookieChangeNotification notification_pointer);
 
+  // Send a CookieChangeNotification over which notification
+  // for all cookie changes will be sent.  When a cookie associated with
+  // the CookieManager changes, a notification will be posted to the
+  // passed pointer.
+  //
+  // TODO(rdsmith): Should this have a filter to register for a lot of
+  // notifications at once?  Maybe combine with the deletion filter?
+  RequestGlobalNotifications(CookieChangeNotification notification_pointer);
+
   // Clone the interface for use somewhere else.  After this call,
   // requests to the same implementation may be posted to the other side
   // of the pipe new_interface was configured on.