[Presentation API] add 'connecting' state to WebPresentationConnectionState

third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp

 // Copyright 2014 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 "modules/presentation/PresentationConnection.h"
 
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "core/dom/DOMArrayBuffer.h"
 #include "core/dom/DOMArrayBufferView.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
+#include "core/dom/ExecutionContextTask.h"
 #include "core/events/Event.h"
 #include "core/events/MessageEvent.h"
 #include "core/fileapi/FileReaderLoader.h"
 #include "core/fileapi/FileReaderLoaderClient.h"
 #include "core/frame/Deprecation.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/UseCounter.h"
 #include "modules/EventTargetModules.h"
 #include "modules/presentation/Presentation.h"
 #include "modules/presentation/PresentationConnectionAvailableEvent.h"
@@ skipping 17 matching lines @@
   Document* document = toDocument(executionContext);
   if (!document->frame())
     return nullptr;
   PresentationController* controller =
       PresentationController::from(*document->frame());
   return controller ? controller->client() : nullptr;
 }
 
 const AtomicString& connectionStateToString(
     WebPresentationConnectionState state) {
+  DEFINE_STATIC_LOCAL(const AtomicString, connectingValue, ("connecting"));
   DEFINE_STATIC_LOCAL(const AtomicString, connectedValue, ("connected"));
   DEFINE_STATIC_LOCAL(const AtomicString, closedValue, ("closed"));
   DEFINE_STATIC_LOCAL(const AtomicString, terminatedValue, ("terminated"));
 
   switch (state) {
+    case WebPresentationConnectionState::Connecting:
+      return connectingValue;
     case WebPresentationConnectionState::Connected:
       return connectedValue;
     case WebPresentationConnectionState::Closed:
       return closedValue;
     case WebPresentationConnectionState::Terminated:
       return terminatedValue;
   }
 
   ASSERT_NOT_REACHED();
   return terminatedValue;
@@ skipping 77 matching lines @@
   Member<PresentationConnection> m_PresentationConnection;
   std::unique_ptr<FileReaderLoader> m_loader;
 };
 
 PresentationConnection::PresentationConnection(LocalFrame* frame,
                                                const String& id,
                                                const KURL& url)
     : DOMWindowProperty(frame),
       m_id(id),
       m_url(url),
       m_state(WebPresentationConnectionState::Connected),

[imcheng, 2016/10/21 23:58:15]

Add a TODO to change this to connecting?

[zhaobin, 2016/10/24 19:42:39]

Done.

       m_binaryType(BinaryTypeBlob) {}
 
 PresentationConnection::~PresentationConnection() {
   ASSERT(!m_blobLoader);
 }
 
 // static
 PresentationConnection* PresentationConnection::take(
     ScriptPromiseResolver* resolver,
     std::unique_ptr<WebPresentationConnectionClient> client,
@@ skipping 205 matching lines @@
     }
     case BinaryTypeArrayBuffer:
       DOMArrayBuffer* buffer = DOMArrayBuffer::create(data, length);
       dispatchEvent(MessageEvent::create(buffer));
       return;
   }
   ASSERT_NOT_REACHED();
 }
 
 void PresentationConnection::close() {
   if (m_state != WebPresentationConnectionState::Connected)

[imcheng, 2016/10/21 23:58:15]

do we allow close()/terminate() on a PresentationConnection that is in
connecting state? It would be worthwhile to double check the current logic to
see if they also apply to connecting state.

[mark a. foltz, 2016/10/24 18:28:16]

close() is allowed.

terminate() is a no-op in a connecting state.  On the one hand, this makes sense
because you don't know if there will be any presentation to terminate.  On the
other hand this is a little unfortunate in that it doesn't allow a site to
cancel a pending presentation request easily.

[zhaobin, 2016/10/24 19:42:39]

Done.

     return;
   WebPresentationClient* client = presentationClient(getExecutionContext());
   if (client)
     client->closeSession(m_url, m_id);
 
   tearDown();
 }
 
 void PresentationConnection::terminate() {
   if (m_state != WebPresentationConnectionState::Connected)
     return;
   WebPresentationClient* client = presentationClient(getExecutionContext());
   if (client)
     client->terminateSession(m_url, m_id);
 
   tearDown();
 }
 
 bool PresentationConnection::matches(
     WebPresentationConnectionClient* client) const {
   return client && m_url == KURL(client->getUrl()) &&
          m_id == static_cast<String>(client->getId());
 }
 
 void PresentationConnection::didChangeState(
     WebPresentationConnectionState state) {
   if (m_state == state)
     return;
 
-  m_state = state;
-  switch (m_state) {
-    case WebPresentationConnectionState::Connected:
-      dispatchEvent(Event::create(EventTypeNames::connect));
-      return;
-    case WebPresentationConnectionState::Terminated:
-      dispatchEvent(Event::create(EventTypeNames::terminate));
-      return;
-    // Closed state is handled in |didClose()|.
-    case WebPresentationConnectionState::Closed:
-      return;
-  }
-  ASSERT_NOT_REACHED();
+  getExecutionContext()->postTask(
+      BLINK_FROM_HERE,
+      createSameThreadTask(&PresentationConnection::didChangeStateInternal,
+                           wrapPersistent(this), state));

[mlamouri (slow - plz ping), 2016/10/21 09:58:12]

Why are you making this asynchronous?

[mark a. foltz, 2016/10/24 18:28:16]

I believe this is to ensure the connect event is dispatched after the start()
Promise is resolved.  Bin, can you add comments explaining why this is async?

[zhaobin, 2016/10/24 19:42:39]

Reverted. We used to make this async to dispatch connect event after start()
promise is resolved, because we invoked PresentationConnection::didChangeState()
in the same task as the task resolving start() promise
(PresentationConnectionCallbacks::onSuccess). Now we notifiy state change with
settimeout in MR (CL/136787485). It is a different task from the task resolving
start() promise. So aysnc is no longer needed.

[zhaobin, 2016/10/24 19:42:39]

reverted.

 }
 
 void PresentationConnection::didClose(
     WebPresentationConnectionCloseReason reason,
     const String& message) {
   if (m_state == WebPresentationConnectionState::Closed)
     return;
 
   m_state = WebPresentationConnectionState::Closed;
   dispatchEvent(PresentationConnectionCloseEvent::create(
@@ skipping 18 matching lines @@
 void PresentationConnection::didFailLoadingBlob(
     FileError::ErrorCode errorCode) {
   ASSERT(!m_messages.isEmpty() && m_messages.first()->type == MessageTypeBlob);
   // FIXME: generate error message?
   // Ignore the current failed blob item and continue with next items.
   m_messages.removeFirst();
   m_blobLoader.clear();
   handleMessageQueue();
 }
 
+void PresentationConnection::didChangeStateInternal(
+    WebPresentationConnectionState state) {
+  if (m_state == state)
+    return;
+
+  m_state = state;
+  switch (m_state) {
+    case WebPresentationConnectionState::Connecting:
+      return;

[mlamouri (slow - plz ping), 2016/10/21 09:58:12]

NOTREACHED() ?

[zhaobin, 2016/10/24 19:42:39]

Done.

+    case WebPresentationConnectionState::Connected:
+      dispatchEvent(Event::create(EventTypeNames::connect));
+      return;
+    case WebPresentationConnectionState::Terminated:
+      dispatchEvent(Event::create(EventTypeNames::terminate));
+      return;
+    // Closed state is handled in |didClose()|.
+    case WebPresentationConnectionState::Closed:
+      return;

[mlamouri (slow - plz ping), 2016/10/21 09:58:12]

NOTREACHED() ?

[zhaobin, 2016/10/24 19:42:39]

Done.

+  }
+  NOTREACHED();
+}
+
 void PresentationConnection::tearDown() {
   // Cancel current Blob loading if any.
   if (m_blobLoader) {
     m_blobLoader->cancel();
     m_blobLoader.clear();
   }
   m_messages.clear();
 }
 
 }  // namespace blink