[Presentation API] Resolve PresentationRequest::reconnect() with existing presentation connection i…

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

 // 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 "modules/presentation/PresentationRequest.h"
 
 #include "bindings/core/v8/CallbackPromiseAdapter.h"
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
@@ skipping 10 matching lines @@
 #include "modules/presentation/PresentationConnectionCallbacks.h"
 #include "modules/presentation/PresentationController.h"
 #include "modules/presentation/PresentationError.h"
 #include "platform/UserGestureIndicator.h"
 
 namespace blink {
 
 namespace {
 
 // TODO(mlamouri): refactor in one common place.
-WebPresentationClient* presentationClient(ExecutionContext* executionContext) {
+PresentationController* presentationController(
+    ExecutionContext* executionContext) {
   DCHECK(executionContext);
 
   Document* document = toDocument(executionContext);
   if (!document->frame())
     return nullptr;
-  PresentationController* controller =
-      PresentationController::from(*document->frame());
+  return PresentationController::from(*document->frame());
+}
+
+WebPresentationClient* presentationClient(ExecutionContext* executionContext) {
+  PresentationController* controller = presentationController(executionContext);
   return controller ? controller->client() : nullptr;
 }
 
 Settings* settings(ExecutionContext* executionContext) {
   DCHECK(executionContext);
 
   Document* document = toDocument(executionContext);
   return document->settings();
 }
 
@@ skipping 90 matching lines @@
   WebVector<WebURL> presentationUrls(static_cast<size_t>(1U));
   presentationUrls[0] = m_url;
   client->startSession(
       presentationUrls,
       WTF::makeUnique<PresentationConnectionCallbacks>(resolver, this));
   return resolver->promise();
 }
 
 ScriptPromise PresentationRequest::reconnect(ScriptState* scriptState,
                                              const String& id) {
+  if (!getExecutionContext()) {

[mlamouri (slow - plz ping), 2017/01/03 12:02:11]

Why are you adding this check? can the ExecutionContext be null here? If I
understand correctly, the current assumes it's never null because
`presentationClient()` does a `DCHECK()` on the `executionContext`.

[zhaobin, 2017/01/03 20:12:07]

toDocument(getExecutionContext())->isSandboxed(...) uses getExecutionContext()
before presentationClient()?

haraken commented in patch set 1:

"Not related to this CL, getExecutionContext() returns null when the context is
detached from the frame."

Thought that he suggests to add a check for getExecutionContext().

+    return ScriptPromise::rejectWithDOMException(
+        scriptState,
+        DOMException::create(InvalidStateError,
+                             "The context is detached from the frame."));
+  }
+
   if (MixedContentChecker::isMixedContent(
           getExecutionContext()->getSecurityOrigin(), m_url)) {
     return rejectWithMixedContentException(scriptState, m_url.getString());
   }
 
   if (toDocument(getExecutionContext())->isSandboxed(SandboxPresentation))
     return rejectWithSandBoxException(scriptState);
 
   WebPresentationClient* client = presentationClient(getExecutionContext());
   if (!client)
     return ScriptPromise::rejectWithDOMException(
         scriptState,
         DOMException::create(
             InvalidStateError,
             "The PresentationRequest is no longer associated to a frame."));
 
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
   // TODO(crbug.com/627655): Accept multiple URLs per PresentationRequest.
   WebVector<WebURL> presentationUrls(static_cast<size_t>(1U));
   presentationUrls[0] = m_url;
-  client->joinSession(
-      presentationUrls, id,
-      WTF::makeUnique<PresentationConnectionCallbacks>(resolver, this));
+
+  PresentationController* controller =
+      presentationController(getExecutionContext());
+  DCHECK(controller);
+
+  PresentationConnection* existingConnection =
+      controller->findExistingConnection(presentationUrls, id);
+  if (existingConnection) {
+    client->joinSession(
+        presentationUrls, id,
+        WTF::makeUnique<ExistingPresentationConnectionCallbacks>(
+            resolver, existingConnection));
+  } else {
+    client->joinSession(
+        presentationUrls, id,
+        WTF::makeUnique<PresentationConnectionCallbacks>(resolver, this));
+  }
   return resolver->promise();
 }
 
 ScriptPromise PresentationRequest::getAvailability(ScriptState* scriptState) {
   if (MixedContentChecker::isMixedContent(
           getExecutionContext()->getSecurityOrigin(), m_url)) {
     return rejectWithMixedContentException(scriptState, m_url.getString());
   }
 
   if (toDocument(getExecutionContext())->isSandboxed(SandboxPresentation))
@@ skipping 27 matching lines @@
   visitor->trace(m_availabilityProperty);
   EventTargetWithInlineData::trace(visitor);
   SuspendableObject::trace(visitor);
 }
 
 PresentationRequest::PresentationRequest(ExecutionContext* executionContext,
                                          const KURL& url)
     : SuspendableObject(executionContext), m_url(url) {}
 
 }  // namespace blink