[PresentationAPI] Ensures only one pending message callback at a time if a frame created multiple s…

content/renderer/presentation/presentation_dispatcher.cc

 // Copyright 2015 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 "content/renderer/presentation/presentation_dispatcher.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "base/logging.h"
@@ skipping 65 matching lines @@
 
 }  // namespace
 
 namespace content {
 
 PresentationDispatcher::PresentationDispatcher(RenderFrame* render_frame)
     : RenderFrameObserver(render_frame),
       controller_(nullptr),
       binding_(this),
       listening_state_(ListeningState::Inactive),
-      last_known_availability_(false) {
+      last_known_availability_(false),
+      listening_for_messages_(false) {
 }
 
 PresentationDispatcher::~PresentationDispatcher() {
   // Controller should be destroyed before the dispatcher when frame is
   // destroyed.
   DCHECK(!controller_);
 }
 
 void PresentationDispatcher::setController(
     blink::WebPresentationController* controller) {
@@ skipping 233 matching lines @@
     bool is_new_navigation,
     bool is_same_page_navigation) {
   blink::WebFrame* frame = render_frame()->GetWebFrame();
   // If not top-level navigation.
   if (frame->parent() || is_same_page_navigation)
     return;
 
   // Remove all pending send message requests.
   MessageRequestQueue empty;
   std::swap(message_request_queue_, empty);
+
+  listening_for_messages_ = false;
 }
 
 void PresentationDispatcher::OnScreenAvailabilityUpdated(bool available) {
   last_known_availability_ = available;
 
   if (listening_state_ == ListeningState::Waiting)
     listening_state_ = ListeningState::Active;
 
   for (auto observer : availability_observers_)
     observer->availabilityChanged(available);
@@ skipping 13 matching lines @@
     return;
 
   // Reset the callback to get the next event.
   presentation_service_->ListenForDefaultSessionStart(base::Bind(
       &PresentationDispatcher::OnDefaultSessionStarted,
       base::Unretained(this)));
 
   if (!session_info.is_null()) {
     controller_->didStartDefaultSession(
         new PresentationSessionClient(session_info.Pass()));
+    StartListenForMessages();
   }
 }
 
 void PresentationDispatcher::OnSessionCreated(
     blink::WebPresentationSessionClientCallbacks* callback,
     presentation::PresentationSessionInfoPtr session_info,
     presentation::PresentationErrorPtr error) {
   DCHECK(callback);
   if (!error.is_null()) {
     DCHECK(session_info.is_null());
     callback->onError(new blink::WebPresentationError(
         GetWebPresentationErrorTypeFromMojo(error->error_type),
         blink::WebString::fromUTF8(error->message)));
     return;
   }
 
   DCHECK(!session_info.is_null());
   callback->onSuccess(new PresentationSessionClient(session_info.Pass()));
+  StartListenForMessages();
+}
+
+void PresentationDispatcher::StartListenForMessages() {
+  if (listening_for_messages_)
+    return;
+
+  listening_for_messages_ = true;
   presentation_service_->ListenForSessionMessages(
       base::Bind(&PresentationDispatcher::OnSessionMessagesReceived,
                  base::Unretained(this)));
 }
 
 void PresentationDispatcher::OnSessionStateChange(
     presentation::PresentationSessionInfoPtr session_info,
     presentation::PresentationSessionState session_state) {
   if (!controller_)
     return;
 
   presentation_service_->ListenForSessionStateChange(base::Bind(
       &PresentationDispatcher::OnSessionStateChange,
       base::Unretained(this)));
 
   DCHECK(!session_info.is_null());
   controller_->didChangeSessionState(
       new PresentationSessionClient(session_info.Pass()),
       GetWebPresentationSessionStateFromMojo(session_state));
 }
 
 void PresentationDispatcher::OnSessionMessagesReceived(
     mojo::Array<presentation::SessionMessagePtr> messages) {
+  if (!listening_for_messages_)

[Kevin M, 2015/07/08 20:16:08]

Add a LOG/DLOG here to call our attention to strange behavior.

[haibinlu, 2015/07/08 20:47:41]

added comments.

this may happen. frame navigated, and MR sent last batches of messages; where
there may be race condition.

+    return;
+
   // When messages is null, there is an error at presentation service side.
-  if (!controller_ || messages.is_null())
+  if (!controller_ || messages.is_null()) {
+    listening_for_messages_ = false;

[Kevin M, 2015/07/08 20:16:07]

LOG?

[haibinlu, 2015/07/08 20:47:41]

null message is expected to indicate messaging error or the end of messaging
channel due to route removal. Log is not really necessary.

     return;
+  }
 
   for (size_t i = 0; i < messages.size(); ++i) {
     if (messages[i]->type ==
         presentation::PresentationMessageType::PRESENTATION_MESSAGE_TYPE_TEXT) {
       controller_->didReceiveSessionTextMessage(
           new PresentationSessionClient(messages[i]->presentation_url,
                                         messages[i]->presentation_id),
           blink::WebString::fromUTF8(messages[i]->message));
     } else {
       // TODO(haibinlu): handle binary message
@@ skipping 38 matching lines @@
   if (should_listen) {
     listening_state_ = ListeningState::Waiting;
     presentation_service_->ListenForScreenAvailability();
   } else {
     listening_state_ = ListeningState::Inactive;
     presentation_service_->StopListeningForScreenAvailability();
   }
 }
 
 }  // namespace content