Make AvailabilityState garbage collected.

Due to lifecycle issues with the presentation API it is easier to
ensure that the objects it creates are garbage collected and we can
copy the lists before iterating on them.

BUG=960785

Change-Id: I06fe61d0465fa36e16a3defa7b06a32b220b05c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1645813
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#666443}

third_party/blink/renderer/modules/presentation/presentation_availability_callbacks.cc

@@ -49,4 +49,8 @@ void PresentationAvailabilityCallbacks::RejectAvailabilityNotSupported() {
   resolver_->Reject(CreateAvailabilityNotSupportedError());
 }
 
+void PresentationAvailabilityCallbacks::Trace(blink::Visitor* visitor) {
+  visitor->Trace(resolver_);
+}
+
 }  // namespace blink

third_party/blink/renderer/modules/presentation/presentation_availability_callbacks.h

@@ -19,9 +19,8 @@ namespace blink {
 // depending on the availability result.
 // TODO(crbug.com/749327): Consider removing this class and have
 // PresentationAvailabilityState use PresentationAvailabilityProperty directly.
-class MODULES_EXPORT PresentationAvailabilityCallbacks {
-  USING_FAST_MALLOC(PresentationAvailabilityCallbacks);
-
+class MODULES_EXPORT PresentationAvailabilityCallbacks
+    : public GarbageCollectedFinalized<PresentationAvailabilityCallbacks> {
  public:
   PresentationAvailabilityCallbacks(PresentationAvailabilityProperty*,
                                     const WTF::Vector<KURL>&);
@@ -30,8 +29,10 @@ class MODULES_EXPORT PresentationAvailabilityCallbacks {
   virtual void Resolve(bool value);
   virtual void RejectAvailabilityNotSupported();
 
+  void Trace(blink::Visitor*);
+
  private:
-  Persistent<PresentationAvailabilityProperty> resolver_;
+  Member<PresentationAvailabilityProperty> resolver_;
   const WTF::Vector<KURL> urls_;
 
   DISALLOW_COPY_AND_ASSIGN(PresentationAvailabilityCallbacks);

third_party/blink/renderer/modules/presentation/presentation_availability_observer.h

@@ -6,6 +6,7 @@
 #define THIRD_PARTY_BLINK_RENDERER_MODULES_PRESENTATION_PRESENTATION_AVAILABILITY_OBSERVER_H_
 
 #include "third_party/blink/public/mojom/presentation/presentation.mojom-blink.h"
+#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
 #include "third_party/blink/renderer/platform/wtf/vector.h"
 
 namespace blink {
@@ -15,7 +16,7 @@ class KURL;
 // PresentationAvailabilityObserver is an interface that is implemented by
 // objects that wish to be notified when there is a presentation display
 // availability change for given URLs.
-class PresentationAvailabilityObserver {
+class PresentationAvailabilityObserver : public GarbageCollectedMixin {
  public:
   virtual ~PresentationAvailabilityObserver() = default;
 

third_party/blink/renderer/modules/presentation/presentation_availability_state.cc

@@ -7,6 +7,7 @@
 #include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/renderer/modules/presentation/presentation_availability_observer.h"
 #include "third_party/blink/renderer/modules/presentation/presentation_controller.h"
+#include "third_party/blink/renderer/platform/heap/persistent.h"
 #include "third_party/blink/renderer/platform/scheduler/public/thread.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
 
@@ -20,7 +21,7 @@ PresentationAvailabilityState::~PresentationAvailabilityState() = default;
 
 void PresentationAvailabilityState::RequestAvailability(
     const Vector<KURL>& urls,
-    std::unique_ptr<PresentationAvailabilityCallbacks> callback) {
+    PresentationAvailabilityCallbacks* callback) {
   auto screen_availability = GetScreenAvailability(urls);
   // Reject Promise if screen availability is unsupported for all URLs.
   if (screen_availability == mojom::blink::ScreenAvailability::DISABLED) {
@@ -28,25 +29,25 @@ void PresentationAvailabilityState::RequestAvailability(
         FROM_HERE,
         WTF::Bind(
             &PresentationAvailabilityCallbacks::RejectAvailabilityNotSupported,
-            std::move(callback)));
+            WrapPersistent(callback)));
     // Do not listen to urls if we reject the promise.
     return;
   }
 
   auto* listener = GetAvailabilityListener(urls);
   if (!listener) {
-    listener = new AvailabilityListener(urls);
+    listener = MakeGarbageCollected<AvailabilityListener>(urls);
     availability_listeners_.emplace_back(listener);
   }
 
   if (screen_availability != mojom::blink::ScreenAvailability::UNKNOWN) {
     Thread::Current()->GetTaskRunner()->PostTask(
         FROM_HERE, WTF::Bind(&PresentationAvailabilityCallbacks::Resolve,
-                             std::move(callback),
+                             WrapPersistent(callback),
                              screen_availability ==
                                  mojom::blink::ScreenAvailability::AVAILABLE));
   } else {
-    listener->availability_callbacks.push_back(std::move(callback));
+    listener->availability_callbacks.push_back(callback);
   }
 
   for (const auto& availability_url : urls)
@@ -58,11 +59,13 @@ void PresentationAvailabilityState::AddObserver(
   const auto& urls = observer->Urls();
   auto* listener = GetAvailabilityListener(urls);
   if (!listener) {
-    listener = new AvailabilityListener(urls);
+    listener = MakeGarbageCollected<AvailabilityListener>(urls);
     availability_listeners_.emplace_back(listener);
   }
 
-  listener->availability_observers.insert(observer);
+  if (listener->availability_observers.Contains(observer))
+    return;
+  listener->availability_observers.push_back(observer);
   for (const auto& availability_url : urls)
     StartListeningToURL(availability_url);
 }
@@ -76,7 +79,10 @@ void PresentationAvailabilityState::RemoveObserver(
     return;
   }
 
-  listener->availability_observers.erase(observer);
+  wtf_size_t slot = listener->availability_observers.Find(observer);
+  if (slot != kNotFound) {
+    listener->availability_observers.EraseAt(slot);
+  }
   for (const auto& availability_url : urls)
     MaybeStopListeningToURL(availability_url);
 
@@ -98,42 +104,40 @@ void PresentationAvailabilityState::UpdateAvailability(
 
   listening_status->last_known_availability = availability;
 
-  std::vector<AvailabilityListener*> modified_listeners;
-  {
-    // Set |iterating_listeners_| so we know not to allow modifications
-    // to |availability_listeners_|.
-    base::AutoReset<bool> iterating(&iterating_listeners_, true);
-    for (auto& listener_ref : availability_listeners_) {
-      auto* listener = listener_ref.get();
-      if (!listener->urls.Contains<KURL>(url))
-        continue;
-
-      auto screen_availability = GetScreenAvailability(listener->urls);
-      DCHECK(screen_availability != mojom::blink::ScreenAvailability::UNKNOWN);
-      for (auto* observer : listener->availability_observers)
-        observer->AvailabilityChanged(screen_availability);
-
-      if (screen_availability == mojom::blink::ScreenAvailability::DISABLED) {
-        for (auto& callback_ptr : listener->availability_callbacks) {
-          callback_ptr->RejectAvailabilityNotSupported();
-        }
-      } else {
-        for (auto& callback_ptr : listener->availability_callbacks) {
-          callback_ptr->Resolve(screen_availability ==
-                                mojom::blink::ScreenAvailability::AVAILABLE);
-        }
-      }
-      listener->availability_callbacks.clear();
+  HeapVector<Member<AvailabilityListener>> listeners = availability_listeners_;
+  for (auto& listener : listeners) {
+    if (!listener->urls.Contains<KURL>(url))
+      continue;
 
-      for (const auto& availability_url : listener->urls)
-        MaybeStopListeningToURL(availability_url);
+    auto screen_availability = GetScreenAvailability(listener->urls);
+    DCHECK(screen_availability != mojom::blink::ScreenAvailability::UNKNOWN);
+    HeapVector<Member<PresentationAvailabilityObserver>> observers =
+        listener->availability_observers;
+    for (auto& observer : observers) {
+      observer->AvailabilityChanged(screen_availability);
+    }
 
-      modified_listeners.push_back(listener);
+    if (screen_availability == mojom::blink::ScreenAvailability::DISABLED) {
+      for (auto& callback_ptr : listener->availability_callbacks) {
+        callback_ptr->RejectAvailabilityNotSupported();
+      }
+    } else {
+      for (auto& callback_ptr : listener->availability_callbacks) {
+        callback_ptr->Resolve(screen_availability ==
+                              mojom::blink::ScreenAvailability::AVAILABLE);
+      }
     }
-  }
+    listener->availability_callbacks.clear();
+
+    for (const auto& availability_url : listener->urls)
+      MaybeStopListeningToURL(availability_url);
 
-  for (auto* listener : modified_listeners)
     TryRemoveAvailabilityListener(listener);
+  }
+}
+
+void PresentationAvailabilityState::Trace(blink::Visitor* visitor) {
+  visitor->Trace(availability_listeners_);
 }
 
 void PresentationAvailabilityState::StartListeningToURL(const KURL& url) {
@@ -157,8 +161,8 @@ void PresentationAvailabilityState::MaybeStopListeningToURL(const KURL& url) {
       continue;
 
     // URL is still observed by some availability object.
-    if (!listener->availability_callbacks.empty() ||
-        !listener->availability_observers.empty()) {
+    if (!listener->availability_callbacks.IsEmpty() ||
+        !listener->availability_observers.IsEmpty()) {
       return;
     }
   }
@@ -216,34 +220,24 @@ PresentationAvailabilityState::GetScreenAvailability(
 
 PresentationAvailabilityState::AvailabilityListener*
 PresentationAvailabilityState::GetAvailabilityListener(
-    const Vector<KURL>& urls) const {
-  auto listener_it = std::find_if(
+    const Vector<KURL>& urls) {
+  auto* listener_it = std::find_if(
       availability_listeners_.begin(), availability_listeners_.end(),
-      [&urls](const std::unique_ptr<AvailabilityListener>& x) {
-        return x->urls == urls;
-      });
-  return listener_it == availability_listeners_.end() ? nullptr
-                                                      : listener_it->get();
+      [&urls](const auto& listener) { return listener->urls == urls; });
+  return listener_it == availability_listeners_.end() ? nullptr : *listener_it;
 }
 
 void PresentationAvailabilityState::TryRemoveAvailabilityListener(
     AvailabilityListener* listener) {
-  if (iterating_listeners_)
-    return;
-
   // URL is still observed by some availability object.
-  if (!listener->availability_callbacks.empty() ||
-      !listener->availability_observers.empty()) {
+  if (!listener->availability_callbacks.IsEmpty() ||
+      !listener->availability_observers.IsEmpty()) {
     return;
   }
 
-  auto listener_it = availability_listeners_.begin();
-  while (listener_it != availability_listeners_.end()) {
-    if (listener_it->get() == listener) {
-      availability_listeners_.erase(listener_it);
-      return;
-    }
-    ++listener_it;
+  wtf_size_t slot = availability_listeners_.Find(listener);
+  if (slot != kNotFound) {
+    availability_listeners_.EraseAt(slot);
   }
 }
 
@@ -266,6 +260,12 @@ PresentationAvailabilityState::AvailabilityListener::AvailabilityListener(
 PresentationAvailabilityState::AvailabilityListener::~AvailabilityListener() =
     default;
 
+void PresentationAvailabilityState::AvailabilityListener::Trace(
+    blink::Visitor* visitor) {
+  visitor->Trace(availability_callbacks);
+  visitor->Trace(availability_observers);
+}
+
 PresentationAvailabilityState::ListeningStatus::ListeningStatus(
     const KURL& availability_url)
     : url(availability_url),

third_party/blink/renderer/modules/presentation/presentation_availability_state.h

@@ -30,9 +30,8 @@ class PresentationAvailabilityObserver;
 // TODO(crbug.com/780109): Improve encapsulation of PresentationAvailability and
 // this class by moving the multiple URL tracking logic to the former, and
 // consolidating this class's APIs to take repeating callbacks.
-class MODULES_EXPORT PresentationAvailabilityState {
-  USING_FAST_MALLOC(PresentationAvailabilityState);
-
+class MODULES_EXPORT PresentationAvailabilityState
+    : public GarbageCollectedFinalized<PresentationAvailabilityState> {
  public:
   explicit PresentationAvailabilityState(mojom::blink::PresentationService*);
   ~PresentationAvailabilityState();
@@ -41,7 +40,7 @@ class MODULES_EXPORT PresentationAvailabilityState {
   // with the determined availability value. The callbacks will only be invoked
   // once and will be deleted afterwards.
   void RequestAvailability(const Vector<KURL>&,
-                           std::unique_ptr<PresentationAvailabilityCallbacks>);
+                           PresentationAvailabilityCallbacks* callbacks);
 
   // Starts/stops listening for availability with the given observer.
   void AddObserver(PresentationAvailabilityObserver*);
@@ -51,6 +50,8 @@ class MODULES_EXPORT PresentationAvailabilityState {
   // callbacks and observers.
   void UpdateAvailability(const KURL&, mojom::blink::ScreenAvailability);
 
+  void Trace(blink::Visitor*);
+
  private:
   enum class ListeningState {
     INACTIVE,
@@ -61,15 +62,20 @@ class MODULES_EXPORT PresentationAvailabilityState {
   // Tracks listeners of presentation displays availability for
   // |availability_urls|. Shared with PresentationRequest objects with the same
   // set of URLs.
-  struct AvailabilityListener {
+  class AvailabilityListener
+      : public GarbageCollectedFinalized<AvailabilityListener> {
+   public:
     explicit AvailabilityListener(const Vector<KURL>& availability_urls);
     ~AvailabilityListener();
 
     const Vector<KURL> urls;
-    std::vector<std::unique_ptr<PresentationAvailabilityCallbacks>>
+    HeapVector<Member<PresentationAvailabilityCallbacks>>
         availability_callbacks;
-    std::set<PresentationAvailabilityObserver*> availability_observers;
+    HeapVector<Member<PresentationAvailabilityObserver>> availability_observers;
 
+    void Trace(blink::Visitor*);
+
+   private:
     DISALLOW_COPY_AND_ASSIGN(AvailabilityListener);
   };
 
@@ -104,7 +110,7 @@ class MODULES_EXPORT PresentationAvailabilityState {
       const Vector<KURL>&) const;
 
   // Returns nullptr if there is no AvailabilityListener for the given URLs.
-  AvailabilityListener* GetAvailabilityListener(const Vector<KURL>&) const;
+  AvailabilityListener* GetAvailabilityListener(const Vector<KURL>&);
 
   // Removes the given listener from |availability_set_| if it has no callbacks
   // and no observers.
@@ -117,15 +123,11 @@ class MODULES_EXPORT PresentationAvailabilityState {
   std::vector<std::unique_ptr<ListeningStatus>> availability_listening_status_;
 
   // Set of AvailabilityListener for known PresentationRequests.
-  std::vector<std::unique_ptr<AvailabilityListener>> availability_listeners_;
+  HeapVector<Member<AvailabilityListener>> availability_listeners_;
 
   // A pointer to PresentationService owned by PresentationController.
   mojom::blink::PresentationService* const presentation_service_;
 
-  // Whether we are iterating listeners, if this state is set we avoid
-  // removing items from |availability_listeners_|.
-  bool iterating_listeners_ = false;
-
   DISALLOW_COPY_AND_ASSIGN(PresentationAvailabilityState);
 };
 

third_party/blink/renderer/modules/presentation/presentation_controller.cc

@@ -55,6 +55,7 @@ PresentationController* PresentationController::FromContext(
 void PresentationController::Trace(blink::Visitor* visitor) {
   visitor->Trace(presentation_);
   visitor->Trace(connections_);
+  visitor->Trace(availability_state_);
   Supplement<LocalFrame>::Trace(visitor);
   ContextLifecycleObserver::Trace(visitor);
 }
@@ -70,11 +71,11 @@ void PresentationController::RegisterConnection(
 
 PresentationAvailabilityState* PresentationController::GetAvailabilityState() {
   if (!availability_state_) {
-    availability_state_.reset(
-        new PresentationAvailabilityState(GetPresentationService().get()));
+    availability_state_ = MakeGarbageCollected<PresentationAvailabilityState>(
+        GetPresentationService().get());
   }
 
-  return availability_state_.get();
+  return availability_state_;
 }
 
 void PresentationController::AddAvailabilityObserver(

third_party/blink/renderer/modules/presentation/presentation_controller.h

@@ -100,7 +100,7 @@ class MODULES_EXPORT PresentationController
       const mojom::blink::PresentationInfo&) const;
 
   // Lazily-instantiated when the page queries for availability.
-  std::unique_ptr<PresentationAvailabilityState> availability_state_;
+  Member<PresentationAvailabilityState> availability_state_;
 
   // The Presentation instance associated with that frame.
   WeakMember<Presentation> presentation_;

third_party/blink/renderer/modules/presentation/presentation_request.cc

@@ -217,7 +217,7 @@ ScriptPromise PresentationRequest::getAvailability(ScriptState* script_state) {
             PresentationAvailabilityProperty::kReady);
 
     controller->GetAvailabilityState()->RequestAvailability(
-        urls_, std::make_unique<PresentationAvailabilityCallbacks>(
+        urls_, MakeGarbageCollected<PresentationAvailabilityCallbacks>(
                    availability_property_, urls_));
   }
   return availability_property_->Promise(script_state->World());