[Presentation API] Handle multiple Presentation URLs in PresentationRequest::getAvailability()

content/renderer/presentation/presentation_dispatcher.cc

Index: content/renderer/presentation/presentation_dispatcher.cc
diff --git a/content/renderer/presentation/presentation_dispatcher.cc b/content/renderer/presentation/presentation_dispatcher.cc
index 29e0f0229ffd9ad023675ce58cdd24e3926d2cd4..741036aed5d69c72f1305269aae4e514aa4714f2 100644
--- a/content/renderer/presentation/presentation_dispatcher.cc
+++ b/content/renderer/presentation/presentation_dispatcher.cc
@@ -92,7 +92,6 @@ GetWebPresentationConnectionCloseReasonFromMojo(
       return blink::WebPresentationConnectionCloseReason::Error;
   }
 }
-
 }  // namespace
 
 namespace content {
@@ -262,57 +261,80 @@ void PresentationDispatcher::terminateSession(
 
 void PresentationDispatcher::getAvailability(
     const blink::WebVector<blink::WebURL>& availabilityUrls,
-    std::unique_ptr<blink::WebPresentationAvailabilityCallbacks> callbacks) {
-  // TODO(mfoltz): Pass all URLs to PresentationService. See crbug.com/627655.
-  const blink::WebURL& availabilityUrl = availabilityUrls[0];
-  AvailabilityStatus* status = nullptr;
-  auto status_it = availability_status_.find(availabilityUrl);
-  if (status_it == availability_status_.end()) {
-    status = new AvailabilityStatus(availabilityUrl);
-    availability_status_[availabilityUrl] = base::WrapUnique(status);
-  } else {
-    status = status_it->second.get();
+    std::unique_ptr<blink::WebPresentationAvailabilityCallbacks> callback) {
+  DCHECK(!availabilityUrls.isEmpty());
+
+  std::vector<GURL> urls;
+  for (const auto& availability_url : availabilityUrls)
+    urls.push_back(availability_url);
+
+  auto screen_availability = GetScreenAvailability(urls);
+  // Reject Promise if screen availability is unsupported for all URLs.
+  if (screen_availability == ScreenAvailability::UNSUPPORTED) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &blink::WebPresentationAvailabilityCallbacks::onError,
+            base::Passed(&callback),
+            blink::WebPresentationError(
+                blink::WebPresentationError::ErrorTypeAvailabilityNotSupported,
+                "Screen availability monitoring not supported")));
+    // Do not listen to urls if we reject the promise.
+    return;
+  }
+
+  auto* status = GetAvailabilityStatus(urls);
+  if (!status) {
+    status = new AvailabilityStatus(urls);
+    availability_set_.insert(base::WrapUnique(status));

[imcheng, 2017/01/17 20:53:17]

Do we ever erase from / clear |availability_set_|?

Looking again, it seems like we don't erase or clear |availability_status_|
before the changes either. I believe the correct behavior should be to clear
them on navigation, and maybe when the object no longer has any
callbacks/observers. WDYT?

[zhaobin, 2017/01/18 03:38:57]

Done.

   }
-  DCHECK(status);
 
-  if (status->listening_state == ListeningState::ACTIVE) {
+  if (screen_availability != ScreenAvailability::UNKNOWN) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
         base::Bind(&blink::WebPresentationAvailabilityCallbacks::onSuccess,
-                   base::Passed(&callbacks), status->last_known_availability));
-    return;
+                   base::Passed(&callback),
+                   screen_availability == ScreenAvailability::AVAILABLE));
+  } else {
+    status->availability_callbacks.Add(std::move(callback));
   }
 
-  status->availability_callbacks.Add(std::move(callbacks));
-  UpdateListeningState(status);
+  for (const auto& availabilityUrl : urls)
+    StartListeningToURL(availabilityUrl);
 }
 
 void PresentationDispatcher::startListening(
     blink::WebPresentationAvailabilityObserver* observer) {
-  // TODO(mfoltz): Pass all URLs to PresentationService. See crbug.com/627655.
-  const blink::WebURL& availabilityUrl = observer->urls()[0];
-  auto status_it = availability_status_.find(availabilityUrl);
-  if (status_it == availability_status_.end()) {
-    DLOG(WARNING) << "Start listening for availability for unknown URL "
-                  << GURL(availabilityUrl);
+  std::vector<GURL> urls;
+  for (const auto& url : observer->urls())
+    urls.push_back(url);
+
+  auto* status = GetAvailabilityStatus(urls);
+  if (!status) {
+    DLOG(WARNING) << "Start listening for availability for unknown URLs.";
     return;
   }
-  status_it->second->availability_observers.insert(observer);
-  UpdateListeningState(status_it->second.get());
+
+  status->availability_observers.insert(observer);
+  for (const auto& availabilityUrl : urls)
+    StartListeningToURL(availabilityUrl);
 }
 
 void PresentationDispatcher::stopListening(
     blink::WebPresentationAvailabilityObserver* observer) {
-  // TODO(mfoltz): Pass all URLs to PresentationService. See crbug.com/627655.
-  const blink::WebURL& availabilityUrl = observer->urls()[0];
-  auto status_it = availability_status_.find(availabilityUrl);
-  if (status_it == availability_status_.end()) {
-    DLOG(WARNING) << "Stop listening for availability for unknown URL "
-                  << GURL(availabilityUrl);
+  std::vector<GURL> urls;
+  for (const auto& url : observer->urls())
+    urls.push_back(url);
+
+  auto* status = GetAvailabilityStatus(urls);
+  if (!status) {
+    DLOG(WARNING) << "Stop listening for availability for unknown URLs.";
     return;
   }
-  status_it->second->availability_observers.erase(observer);
-  UpdateListeningState(status_it->second.get());
+
+  status->availability_observers.erase(observer);
+  for (const auto& availabilityUrl : urls)
+    StopListeningToURL(availabilityUrl);
 }
 
 void PresentationDispatcher::setDefaultPresentationUrls(
@@ -351,47 +373,81 @@ void PresentationDispatcher::OnDestruct() {
 
 void PresentationDispatcher::OnScreenAvailabilityUpdated(const GURL& url,
                                                          bool available) {
-  auto status_it = availability_status_.find(url);
-  if (status_it == availability_status_.end())
+  auto* listening_status = GetListeningStatus(url);
+  if (!listening_status)
     return;
-  AvailabilityStatus* status = status_it->second.get();
-  DCHECK(status);
 
-  if (status->listening_state == ListeningState::WAITING)
-    status->listening_state = ListeningState::ACTIVE;
+  if (listening_status->listening_state == ListeningState::WAITING)
+    listening_status->listening_state = ListeningState::ACTIVE;
+
+  auto new_screen_availability = available ? ScreenAvailability::AVAILABLE
+                                           : ScreenAvailability::UNAVAILABLE;
+  if (listening_status->last_known_availability == new_screen_availability)
+    return;
 
-  for (auto* observer : status->availability_observers)
-    observer->availabilityChanged(available);
+  listening_status->last_known_availability = new_screen_availability;
 
-  for (AvailabilityCallbacksMap::iterator iter(&status->availability_callbacks);
-       !iter.IsAtEnd(); iter.Advance()) {
-    iter.GetCurrentValue()->onSuccess(available);
+  for (auto& status : availability_set_) {
+    if (!base::ContainsValue(status->urls, url))
+      continue;
+    auto screen_availability = GetScreenAvailability(status->urls);

[imcheng, 2017/01/17 20:53:17]

nit: blank line above this

[zhaobin, 2017/01/18 03:38:57]

Done.

+    DCHECK(screen_availability == ScreenAvailability::AVAILABLE ||
+           screen_availability == ScreenAvailability::UNAVAILABLE);
+    bool is_available = (screen_availability == ScreenAvailability::AVAILABLE);
+
+    for (auto* observer : status->availability_observers)
+      observer->availabilityChanged(is_available);
+
+    for (AvailabilityCallbacksMap::iterator iter(
+             &status->availability_callbacks);
+         !iter.IsAtEnd(); iter.Advance()) {
+      iter.GetCurrentValue()->onSuccess(is_available);
+    }
+    status->availability_callbacks.Clear();

[imcheng, 2017/01/17 20:53:17]

The original code had some logic to stop listening after this when there are no
more observers/callbacks.

[zhaobin, 2017/01/18 03:38:57]

Done.

   }
-  status->last_known_availability = available;
-  status->availability_callbacks.Clear();
-  UpdateListeningState(status);
 }
 
 void PresentationDispatcher::OnScreenAvailabilityNotSupported(const GURL& url) {
-  auto status_it = availability_status_.find(url);
-  if (status_it == availability_status_.end())
+  auto* listening_status = GetListeningStatus(url);
+  if (!listening_status)
     return;
-  AvailabilityStatus* status = status_it->second.get();
-  DCHECK(status);
-  DCHECK(status->listening_state == ListeningState::WAITING);
-
-  const blink::WebString& not_supported_error = blink::WebString::fromUTF8(
-      "getAvailability() isn't supported at the moment. It can be due to "
-      "a permanent or temporary system limitation. It is recommended to "
-      "try to blindly start a session in that case.");
-  for (AvailabilityCallbacksMap::iterator iter(&status->availability_callbacks);
-       !iter.IsAtEnd(); iter.Advance()) {
-    iter.GetCurrentValue()->onError(blink::WebPresentationError(
-        blink::WebPresentationError::ErrorTypeAvailabilityNotSupported,
-        not_supported_error));
+
+  if (listening_status->listening_state == ListeningState::WAITING)
+    listening_status->listening_state = ListeningState::ACTIVE;
+
+  if (listening_status->last_known_availability ==
+      ScreenAvailability::UNSUPPORTED)
+    return;
+
+  listening_status->last_known_availability = ScreenAvailability::UNSUPPORTED;
+
+  for (auto& status : availability_set_) {
+    if (!base::ContainsValue(status->urls, url))
+      continue;
+
+    // ScreenAvailabilityNotSupported should be a browser side setting, which
+    // means all urls in PresentationAvailability should report NotSupported.
+    // It is not possible to change listening status from Available or
+    // Unavailable to NotSupported. No need to update observer.
+    auto screen_availability = GetScreenAvailability(status->urls);
+    DCHECK_EQ(screen_availability, ScreenAvailability::UNSUPPORTED);
+
+    const blink::WebString& not_supported_error = blink::WebString::fromUTF8(

[imcheng, 2017/01/17 20:53:17]

nit: put this outside the for-loop.

[zhaobin, 2017/01/18 03:38:57]

Done.

+        "getAvailability() isn't supported at the moment. It can be due to "
+        "a permanent or temporary system limitation. It is recommended to "
+        "try to blindly start a session in that case.");
+    for (AvailabilityCallbacksMap::iterator iter(
+             &status->availability_callbacks);
+         !iter.IsAtEnd(); iter.Advance()) {
+      iter.GetCurrentValue()->onError(blink::WebPresentationError(
+          blink::WebPresentationError::ErrorTypeAvailabilityNotSupported,
+          not_supported_error));
+    }
+    status->availability_callbacks.Clear();
+
+    for (const auto& availability_url : status->urls)
+      StopListeningToURL(availability_url);
   }
-  status->availability_callbacks.Clear();
-  UpdateListeningState(status);
 }
 
 void PresentationDispatcher::OnDefaultSessionStarted(
@@ -499,22 +555,90 @@ void PresentationDispatcher::ConnectToPresentationServiceIfNeeded() {
   presentation_service_->SetClient(binding_.CreateInterfacePtrAndBind());
 }
 
-void PresentationDispatcher::UpdateListeningState(AvailabilityStatus* status) {
-  bool should_listen = !status->availability_callbacks.IsEmpty() ||
-                       !status->availability_observers.empty();
-  bool is_listening = status->listening_state != ListeningState::INACTIVE;
+void PresentationDispatcher::StartListeningToURL(const GURL& url) {
+  auto* listening_status = GetListeningStatus(url);
+  if (!listening_status) {
+    listening_status = new ListeningStatus(url);
+    listening_status_.insert(
+        std::make_pair(url, base::WrapUnique(listening_status)));
+  }
 
-  if (should_listen == is_listening)
+  // Already listening.
+  if (listening_status->listening_state != ListeningState::INACTIVE)
     return;
 
   ConnectToPresentationServiceIfNeeded();
-  if (should_listen) {
-    status->listening_state = ListeningState::WAITING;
-    presentation_service_->ListenForScreenAvailability(status->url);
-  } else {
-    status->listening_state = ListeningState::INACTIVE;
-    presentation_service_->StopListeningForScreenAvailability(status->url);
+  listening_status->listening_state = ListeningState::WAITING;
+  presentation_service_->ListenForScreenAvailability(url);
+}
+
+void PresentationDispatcher::StopListeningToURL(const GURL& url) {
+  for (const auto& status : availability_set_) {
+    if (!base::ContainsValue(status->urls, url))
+      continue;
+
+    // URL is still observed by some availability object.
+    if (!status->availability_callbacks.IsEmpty() ||
+        !status->availability_observers.empty())
+      return;
   }
+
+  auto* listening_status = GetListeningStatus(url);
+  DCHECK(listening_status);

[imcheng, 2017/01/17 20:53:17]

This will still blow up in release builds. Suggest either turning this into a
CHECK or handle nullptr gracefully.

[zhaobin, 2017/01/18 03:38:57]

Done.

+
+  if (listening_status->listening_state == ListeningState::INACTIVE)
+    return;
+
+  ConnectToPresentationServiceIfNeeded();
+  listening_status->listening_state = ListeningState::INACTIVE;
+  presentation_service_->StopListeningForScreenAvailability(url);
+}
+
+PresentationDispatcher::ListeningStatus*
+PresentationDispatcher::GetListeningStatus(const GURL& url) {
+  ListeningStatus* status = nullptr;
+  auto status_it = listening_status_.find(url);
+  if (status_it != listening_status_.end())
+    status = status_it->second.get();
+  return status;
+}
+
+PresentationDispatcher::AvailabilityStatus*
+PresentationDispatcher::GetAvailabilityStatus(const std::vector<GURL>& urls) {
+  auto status_it =
+      std::find_if(availability_set_.begin(), availability_set_.end(),
+                   [&urls](const std::unique_ptr<AvailabilityStatus>& x) {
+                     return x->urls == urls;
+                   });
+
+  if (status_it == availability_set_.end())

[imcheng, 2017/01/17 20:53:17]

nit: replace with:

return status_it == availability_set_.end() ? nullptr : status_it->get();

[zhaobin, 2017/01/18 03:38:57]

Done.

+    return nullptr;
+
+  return status_it->get();
+}
+
+PresentationDispatcher::ScreenAvailability
+PresentationDispatcher::GetScreenAvailability(const std::vector<GURL>& urls) {
+  std::map<ScreenAvailability, size_t> counters;
+
+  for (const auto& url : urls) {
+    if (!base::ContainsKey(listening_status_, url)) {
+      counters[ScreenAvailability::UNKNOWN]++;
+    } else {
+      auto url_availability = listening_status_[url]->last_known_availability;
+      counters[url_availability]++;
+    }
+  }
+
+  if (counters[ScreenAvailability::AVAILABLE] > 0)
+    return ScreenAvailability::AVAILABLE;
+  if (counters[ScreenAvailability::UNSUPPORTED] > 0)
+    return ScreenAvailability::UNSUPPORTED;
+  if (counters[ScreenAvailability::UNAVAILABLE] > 0)
+    return ScreenAvailability::UNAVAILABLE;
+
+  DCHECK_EQ(counters[ScreenAvailability::UNKNOWN], urls.size());

[imcheng, 2017/01/17 20:53:17]

nit: operator[] contains side effects so we avoid calling it inside a DCHECK.

[zhaobin, 2017/01/18 03:38:57]

Done.

+  return ScreenAvailability::UNKNOWN;
 }
 
 PresentationDispatcher::SendMessageRequest::SendMessageRequest(
@@ -565,12 +689,17 @@ PresentationDispatcher::CreateSendBinaryMessageRequest(
 }
 
 PresentationDispatcher::AvailabilityStatus::AvailabilityStatus(
+    const std::vector<GURL>& availability_urls)
+    : urls(availability_urls) {}
+
+PresentationDispatcher::AvailabilityStatus::~AvailabilityStatus() {}
+
+PresentationDispatcher::ListeningStatus::ListeningStatus(
     const GURL& availability_url)
     : url(availability_url),
-      last_known_availability(false),
+      last_known_availability(ScreenAvailability::UNKNOWN),
       listening_state(ListeningState::INACTIVE) {}
 
-PresentationDispatcher::AvailabilityStatus::~AvailabilityStatus() {
-}
+PresentationDispatcher::ListeningStatus::~ListeningStatus() {}
 
 }  // namespace content