Fixed gcdPrivate behavior if message is sent before confirmation code.

BUG=419824
TEST=session.js

Review URL: https://codereview.chromium.org/623833004

Cr-Commit-Position: refs/heads/master@{#298421}

5 files changed, 44 insertions, 17 deletions

diff --git a/chrome/browser/extensions/api/gcd_private/gcd_private_api.cc b/chrome/browser/extensions/api/gcd_private/gcd_private_api.cc
index 8a342ff..9da1296 100644
--- a/chrome/browser/extensions/api/gcd_private/gcd_private_api.cc
+++ b/chrome/browser/extensions/api/gcd_private/gcd_private_api.cc
@@ -201,11 +201,13 @@ class GcdPrivateRequest : public local_discovery::PrivetV3Session::Request {
   // local_discovery::PrivetV3Session::Request implementation.
   virtual std::string GetName() override;
   virtual const base::DictionaryValue& GetInput() override;
-  virtual void OnError(
-      local_discovery::PrivetURLFetcher::ErrorType error) override;
+  virtual void OnError() override;
   virtual void OnParsedJson(const base::DictionaryValue& value,
                             bool has_error) override;
 
+  void RunCallback(gcd_private::Status status,
+                   const base::DictionaryValue& value);
+
  private:
   std::string api_;
   scoped_ptr<base::DictionaryValue> input_;
@@ -507,20 +509,25 @@ const base::DictionaryValue& GcdPrivateRequest::GetInput() {
   return *input_;
 }
 
-void GcdPrivateRequest::OnError(
-    local_discovery::PrivetURLFetcher::ErrorType error) {
-  callback_.Run(gcd_private::STATUS_CONNECTIONERROR, base::DictionaryValue());
-
+void GcdPrivateRequest::OnError() {
+  RunCallback(gcd_private::STATUS_CONNECTIONERROR, base::DictionaryValue());
   session_holder_->DeleteRequest(this);
 }
 
 void GcdPrivateRequest::OnParsedJson(const base::DictionaryValue& value,
                                      bool has_error) {
-  callback_.Run(gcd_private::STATUS_SUCCESS, value);
-
+  RunCallback(gcd_private::STATUS_SUCCESS, value);
   session_holder_->DeleteRequest(this);
 }
 
+void GcdPrivateRequest::RunCallback(gcd_private::Status status,
+                                    const base::DictionaryValue& value) {
+  if (callback_.is_null())
+    return;
+  callback_.Run(status, value);
+  callback_.Reset();
+}
+
 GcdPrivateSessionHolder::GcdPrivateSessionHolder(
     const std::string& ip_address,
     int port,
@@ -581,16 +588,25 @@ void GcdPrivateSessionHolder::DeleteRequest(GcdPrivateRequest* request) {
 void GcdPrivateSessionHolder::OnSetupConfirmationNeeded(
     const std::string& confirmation_code,
     gcd_private::ConfirmationType confirmation_type) {
+  if (confirm_callback_.is_null())
+    return;
   confirm_callback_.Run(
       gcd_private::STATUS_SUCCESS, confirmation_code, confirmation_type);
-
   confirm_callback_.Reset();
 }
 
 void GcdPrivateSessionHolder::OnSessionStatus(gcd_private::Status status) {
-  session_established_callback_.Run(status);
-
-  session_established_callback_.Reset();
+  if (!session_established_callback_.is_null()) {
+    session_established_callback_.Run(status);
+    session_established_callback_.Reset();
+  }
+  // Fail all requests created before session established.
+  RequestVector tmp_requests;
+  tmp_requests.swap(requests_);
+  for (GcdPrivateRequest* request : tmp_requests) {
+    request->RunCallback(gcd_private::STATUS_SESSIONERROR,
+                         base::DictionaryValue());
+  }
 }
 
 GcdPrivateAPI::GcdPrivateAPI(content::BrowserContext* context)
diff --git a/chrome/browser/local_discovery/privetv3_session.cc b/chrome/browser/local_discovery/privetv3_session.cc
index 2a2775f..cb619a9 100644
--- a/chrome/browser/local_discovery/privetv3_session.cc
+++ b/chrome/browser/local_discovery/privetv3_session.cc
@@ -69,7 +69,7 @@ void PrivetV3Session::FetcherDelegate::OnNeedPrivetToken(
 void PrivetV3Session::FetcherDelegate::OnError(
     PrivetURLFetcher* fetcher,
     PrivetURLFetcher::ErrorType error) {
-  request_->OnError(error);
+  request_->OnError();
 }
 
 void PrivetV3Session::FetcherDelegate::OnParsedJson(
@@ -118,7 +118,11 @@ void PrivetV3Session::ConfirmCode(const std::string& code) {
 }
 
 void PrivetV3Session::StartRequest(Request* request) {
-  CHECK(code_confirmed_);
+  if (!code_confirmed_) {
+    delegate_->OnSessionStatus(
+        extensions::api::gcd_private::STATUS_SESSIONERROR);
+    return;
+  }
 
   request->fetcher_delegate_.reset(
       new FetcherDelegate(weak_ptr_factory_.GetWeakPtr(), request));
diff --git a/chrome/browser/local_discovery/privetv3_session.h b/chrome/browser/local_discovery/privetv3_session.h
index 91771b1..c24f7ab 100644
--- a/chrome/browser/local_discovery/privetv3_session.h
+++ b/chrome/browser/local_discovery/privetv3_session.h
@@ -48,7 +48,7 @@ class PrivetV3Session {
 
     virtual std::string GetName() = 0;
     virtual const base::DictionaryValue& GetInput() = 0;
-    virtual void OnError(PrivetURLFetcher::ErrorType error) = 0;
+    virtual void OnError() = 0;
     virtual void OnParsedJson(const base::DictionaryValue& value,
                               bool has_error) = 0;
 
diff --git a/chrome/browser/local_discovery/privetv3_setup_flow.cc b/chrome/browser/local_discovery/privetv3_setup_flow.cc
index 9f3554f..8750128 100644
--- a/chrome/browser/local_discovery/privetv3_setup_flow.cc
+++ b/chrome/browser/local_discovery/privetv3_setup_flow.cc
@@ -24,7 +24,7 @@ class SetupRequest : public PrivetV3Session::Request {
   virtual std::string GetName() override { return "/privet/v3/setup/start"; }
   virtual const base::DictionaryValue& GetInput() override;
 
-  virtual void OnError(PrivetURLFetcher::ErrorType error) override;
+  virtual void OnError() override;
   virtual void OnParsedJson(const base::DictionaryValue& value,
                             bool has_error) override;
 
@@ -49,7 +49,7 @@ const base::DictionaryValue& SetupRequest::GetInput() {
   return input_;
 }
 
-void SetupRequest::OnError(PrivetURLFetcher::ErrorType error) {
+void SetupRequest::OnError() {
   setup_flow_->OnSetupError();
 }
 
diff --git a/chrome/test/data/extensions/api_test/gcd_private/api/session.js b/chrome/test/data/extensions/api_test/gcd_private/api/session.js
index 48d981e..e4d2f9b 100644
--- a/chrome/test/data/extensions/api_test/gcd_private/api/session.js
+++ b/chrome/test/data/extensions/api_test/gcd_private/api/session.js
@@ -10,6 +10,9 @@ onload = function() {
         chrome.test.assertEq("1234", confirmationInfo.code);
         chrome.test.assertEq("displayCode", confirmationInfo.type);
 
+        chrome.gcdPrivate.sendMessage(sessionId, "/privet/ping", {},
+                                      onMessageSentFail);
+
         chrome.gcdPrivate.confirmCode(sessionId,
                                       "1234",
                                       onSessionEstablished.bind(null,
@@ -23,6 +26,10 @@ onload = function() {
                                       onMessageSent);
       }
 
+      function onMessageSentFail(status, output) {
+        chrome.test.assertEq("sessionError", status);
+      }
+
       function onMessageSent(status, output) {
         chrome.test.assertEq("success", status);
         chrome.test.assertEq("pong", output.response);