base::ObserverList::has_observers()

Which does not return false positive unlike might_have_observers().

Mark might_have_observers deprecated. I'll migrate the callers in
a seperate CL.

Bug: 1155308
Test: updated existing unit tests.
Change-Id: I3dcab10c64ac3e6a3a0aacbb68844ef09ab9bef5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2572885Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833582}

base/observer_list.h

@@ -273,6 +273,7 @@ class ObserverList {
       NOTREACHED() << "Observers can only be added once!";
       return;
     }
+    observers_count_++;
     observers_.emplace_back(ObserverStorageType(obs));
   }
 
@@ -284,7 +285,8 @@ class ObserverList {
         observers_, [obs](const auto& o) { return o.IsEqual(obs); });
     if (it == observers_.end())
       return;
-
+    if (!it->IsMarkedForRemoval())
+      observers_count_--;
     if (live_iterators_.empty()) {
       observers_.erase(it);
     } else {
@@ -314,8 +316,13 @@ class ObserverList {
       for (auto& observer : observers_)
         observer.MarkForRemoval();
     }
+    observers_count_ = 0;
   }
 
+  bool has_observers() const { return observers_count_ > 0; }
+
+  // Deprecated: use |has_observers()|.
+  // TODO(1155308): migrate all callers and make this test only.
   bool might_have_observers() const { return !observers_.empty(); }
 
  private:
@@ -334,6 +341,8 @@ class ObserverList {
 
   base::LinkedList<internal::WeakLinkNode<ObserverList>> live_iterators_;
 
+  size_t observers_count_{0};
+
   const ObserverListPolicy policy_;
 
   SEQUENCE_CHECKER(iteration_sequence_checker_);

base/observer_list_unittest.cc

@@ -364,6 +364,7 @@ TYPED_TEST(ObserverListTest, CompactsWhenNoActiveIterator) {
   EXPECT_TRUE(col.HasObserver(&a));
   EXPECT_FALSE(col.HasObserver(&c));
 
+  EXPECT_TRUE(col.has_observers());
   EXPECT_TRUE(col.might_have_observers());
 
   using It = typename ObserverListConstFoo::const_iterator;
@@ -379,44 +380,55 @@ TYPED_TEST(ObserverListTest, CompactsWhenNoActiveIterator) {
     EXPECT_EQ(itb, it);
     EXPECT_EQ(++it, col.end());
 
+    EXPECT_TRUE(col.has_observers());
     EXPECT_TRUE(col.might_have_observers());
     EXPECT_EQ(&*ita, &a);
     EXPECT_EQ(&*itb, &b);
 
     ol.RemoveObserver(&a);
+    EXPECT_TRUE(col.has_observers());
     EXPECT_TRUE(col.might_have_observers());
     EXPECT_FALSE(col.HasObserver(&a));
     EXPECT_EQ(&*itb, &b);
 
     ol.RemoveObserver(&b);
+    EXPECT_FALSE(col.has_observers());
     EXPECT_TRUE(col.might_have_observers());
     EXPECT_FALSE(col.HasObserver(&a));
     EXPECT_FALSE(col.HasObserver(&b));
 
     it = It();
     ita = It();
+    EXPECT_FALSE(col.has_observers());
     EXPECT_TRUE(col.might_have_observers());
     ita = itb;
     itb = It();
+    EXPECT_FALSE(col.has_observers());
     EXPECT_TRUE(col.might_have_observers());
     ita = It();
+    EXPECT_FALSE(col.has_observers());
     EXPECT_FALSE(col.might_have_observers());
   }
 
   ol.AddObserver(&a);
   ol.AddObserver(&b);
+  EXPECT_TRUE(col.has_observers());
   EXPECT_TRUE(col.might_have_observers());
   ol.Clear();
+  EXPECT_FALSE(col.has_observers());
   EXPECT_FALSE(col.might_have_observers());
 
   ol.AddObserver(&a);
   ol.AddObserver(&b);
+  EXPECT_TRUE(col.has_observers());
   EXPECT_TRUE(col.might_have_observers());
   {
     const It it = col.begin();
     ol.Clear();
+    EXPECT_FALSE(col.has_observers());
     EXPECT_TRUE(col.might_have_observers());
   }
+  EXPECT_FALSE(col.has_observers());
   EXPECT_FALSE(col.might_have_observers());
 }
 
@@ -1001,6 +1013,7 @@ TEST_F(CheckedObserverListTest, CheckedObserver) {
     // On the non-death fork, no UAF occurs since the deleted observer is never
     // notified, but also the observer list still has |l2| in it. Check that.
     list->RemoveObserver(&l1);
+    EXPECT_TRUE(list->has_observers());
     EXPECT_TRUE(list->might_have_observers());
 
     // Now (in the non-death fork()) there's a problem. To delete |it|, we need