GCP2.0 Device: Instant AccessToken update on AuthFailed.

cloud_print/gcp20/prototype/printer.cc

Index: cloud_print/gcp20/prototype/printer.cc
diff --git a/cloud_print/gcp20/prototype/printer.cc b/cloud_print/gcp20/prototype/printer.cc
index 4583aa70b4ebfc3ff561f71ac6290afda17b4fe5..7155dd686ffc8b62dcc0616c8c43949bb545584a 100644
--- a/cloud_print/gcp20/prototype/printer.cc
+++ b/cloud_print/gcp20/prototype/printer.cc
@@ -39,9 +39,9 @@ const char kPrinterDescription[] = "Printer emulator";
 
 const char kUserConfirmationTitle[] = "Confirm registration: type 'y' if you "
                                       "agree and any other to discard\n";
-const int64 kUserConfirmationTimeout = 30;  // in seconds
-
-const uint32 kReconnectTimeout = 5;  // in seconds
+const int kUserConfirmationTimeout = 30;  // in seconds
+const int kRegistrationTimeout = 60;  // in seconds
+const int kReconnectTimeout = 5;  // in seconds
 
 const double kTimeToNextAccessTokenUpdate = 0.8;  // relatively to living time.
 
@@ -184,9 +184,8 @@ void Printer::Stop() {
 
 void Printer::OnAuthError() {
   LOG(ERROR) << "Auth error occurred";
-  access_token_update_ = base::Time::Now();
-  ChangeState(OFFLINE);
-  // TODO(maksymb): Implement *instant* updating of access_token.
+  access_token_update_ = base::Time::UnixEpoch();

[Vitaly Buka (NO REVIEWS), 2013/08/15 17:48:06]

Why not now?

[maksymb, 2013/08/15 18:39:13]

UnixEpoch() is definitely in the past, Now() is only now and when we check
whether we need to update |access_token| in TryConnect():
    if (access_token_update_ < base::Time::Now()) {
      requester_->UpdateAccesstoken(reg_info_.refresh_token);
it may be called /*in theory*/ in the same microsecond and won't be updated.
Another solution is to change comparison from < to <=. Is this solution more
readable? UnixEpoch() variant is stable enough and reader doesn't have to know
exact implementation of comparison (< or <=) to be sure it will work.

[Vitaly Buka (NO REVIEWS), 2013/08/15 20:17:06]

= base::Time::Time();

On 2013/08/15 18:39:13, maksymb wrote:

[maksymb, 2013/08/15 20:23:00]

Done.

+  FallOffline(true);
 }
 
 std::string Printer::GetAccessToken() {
@@ -195,6 +194,15 @@ std::string Printer::GetAccessToken() {
 
 PrivetHttpServer::RegistrationErrorStatus Printer::RegistrationStart(
     const std::string& user) {
+  CheckRegistrationExpiration();
+
+  RegistrationInfo::ConfirmationState conf_state = reg_info_.confirmation_state;
+  if (reg_info_.state == RegistrationInfo::DEV_REG_REGISTRATION_ERROR ||
+      conf_state == RegistrationInfo::CONFIRMATION_TIMEOUT ||
+      conf_state == RegistrationInfo::CONFIRMATION_DISCARDED) {
+    reg_info_ = RegistrationInfo();
+  }
+
   PrivetHttpServer::RegistrationErrorStatus status = CheckCommonRegErrors(user);
   if (status != PrivetHttpServer::REG_ERROR_OK)
     return status;
@@ -202,6 +210,8 @@ PrivetHttpServer::RegistrationErrorStatus Printer::RegistrationStart(
   if (reg_info_.state != RegistrationInfo::DEV_REG_UNREGISTERED)
     return PrivetHttpServer::REG_ERROR_INVALID_ACTION;
 
+  UpdateRegistrationExpiration();
+
   reg_info_ = RegistrationInfo();
   reg_info_.user = user;
   reg_info_.state = RegistrationInfo::DEV_REG_REGISTRATION_STARTED;
@@ -242,6 +252,8 @@ PrivetHttpServer::RegistrationErrorStatus Printer::RegistrationGetClaimToken(
   if (reg_info_.confirmation_state != RegistrationInfo::CONFIRMATION_CONFIRMED)
     return ConfirmationToRegistrationError(reg_info_.confirmation_state);
 
+  UpdateRegistrationExpiration();
+
   // If reply wasn't received yet, reply with |pending_user_action| error.
   if (reg_info_.state == RegistrationInfo::DEV_REG_REGISTRATION_STARTED)
     return PrivetHttpServer::REG_ERROR_PENDING_USER_ACTION;
@@ -268,6 +280,8 @@ PrivetHttpServer::RegistrationErrorStatus Printer::RegistrationComplete(
     return PrivetHttpServer::REG_ERROR_INVALID_ACTION;
   }
 
+  UpdateRegistrationExpiration();
+
   if (reg_info_.confirmation_state != RegistrationInfo::CONFIRMATION_CONFIRMED)
     return ConfirmationToRegistrationError(reg_info_.confirmation_state);
 
@@ -289,6 +303,8 @@ PrivetHttpServer::RegistrationErrorStatus Printer::RegistrationCancel(
   if (reg_info_.state == RegistrationInfo::DEV_REG_UNREGISTERED)
     return PrivetHttpServer::REG_ERROR_INVALID_ACTION;
 
+  InvalidateRegistrationExpiration();
+
   reg_info_ = RegistrationInfo();
   requester_.reset(new CloudPrintRequester(GetTaskRunner(), this));
 
@@ -296,13 +312,15 @@ PrivetHttpServer::RegistrationErrorStatus Printer::RegistrationCancel(
 }
 
 void Printer::GetRegistrationServerError(std::string* description) {
-  DCHECK_EQ(reg_info_.state, RegistrationInfo::DEV_REG_REGISTRATION_ERROR) <<
-      "Method shouldn't be called when not needed.";
+  DCHECK_EQ(reg_info_.state, RegistrationInfo::DEV_REG_REGISTRATION_ERROR)
+      << "Method shouldn't be called when not needed.";
 
   *description = reg_info_.error_description;
 }
 
 void Printer::CreateInfo(PrivetHttpServer::DeviceInfo* info) {
+  CheckRegistrationExpiration();
+
   // TODO(maksymb): Replace "text" with constants.
 
   *info = PrivetHttpServer::DeviceInfo();
@@ -345,9 +363,11 @@ void Printer::OnRegistrationStartResponseParsed(
   reg_info_.complete_invite_url = complete_invite_url;
 }
 
-void Printer::OnGetAuthCodeResponseParsed(const std::string& refresh_token,
-                                          const std::string& access_token,
-                                          int access_token_expires_in_seconds) {
+void Printer::OnRegistrationFinished(const std::string& refresh_token,
+                                     const std::string& access_token,
+                                     int access_token_expires_in_seconds) {
+  InvalidateRegistrationExpiration();
+
   reg_info_.state = RegistrationInfo::DEV_REG_REGISTERED;
   reg_info_.refresh_token = refresh_token;
   RememberAccessToken(access_token, access_token_expires_in_seconds);
@@ -380,20 +400,19 @@ void Printer::OnRegistrationError(const std::string& description) {
   LOG(ERROR) << "server_error: " << description;
 
   // TODO(maksymb): Implement waiting after error and timeout of registration.
-  reg_info_.state = RegistrationInfo::DEV_REG_REGISTRATION_ERROR;
-  reg_info_.error_description = description;
+  SetRegistrationError(description);
 }
 
 void Printer::OnNetworkError() {
   VLOG(3) << "Function: " << __FUNCTION__;
-  ChangeState(OFFLINE);
+  FallOffline(false);
 }
 
 void Printer::OnServerError(const std::string& description) {
   VLOG(3) << "Function: " << __FUNCTION__;
   LOG(ERROR) << "Server error: " << description;
 
-  ChangeState(OFFLINE);
+  FallOffline(false);
 }
 
 void Printer::OnPrintJobsAvailable(const std::vector<Job>& jobs) {
@@ -439,7 +458,7 @@ void Printer::OnXmppAuthError() {
 }
 
 void Printer::OnXmppNetworkError() {
-  ChangeState(OFFLINE);
+  FallOffline(false);
 }
 
 void Printer::OnXmppNewPrintJob(const std::string& device_id) {
@@ -542,9 +561,17 @@ void Printer::RememberAccessToken(const std::string& access_token,
   SaveToFile();
 }
 
+void Printer::SetRegistrationError(const std::string& description) {
+  DCHECK(!IsRegistered());
+  reg_info_.state = RegistrationInfo::DEV_REG_REGISTRATION_ERROR;
+  reg_info_.error_description = description;
+}
+
 PrivetHttpServer::RegistrationErrorStatus Printer::CheckCommonRegErrors(
-    const std::string& user) const {
+    const std::string& user) {
+  CheckRegistrationExpiration();
   DCHECK(!IsRegistered());
+  DCHECK(connection_state_ == ONLINE);
 
   if (reg_info_.state != RegistrationInfo::DEV_REG_UNREGISTERED &&
       user != reg_info_.user) {
@@ -554,10 +581,14 @@ PrivetHttpServer::RegistrationErrorStatus Printer::CheckCommonRegErrors(
   if (reg_info_.state == RegistrationInfo::DEV_REG_REGISTRATION_ERROR)
     return PrivetHttpServer::REG_ERROR_SERVER_ERROR;
 
+  DCHECK(connection_state_ == ONLINE);
+
   return PrivetHttpServer::REG_ERROR_OK;
 }
 
 void Printer::WaitUserConfirmation(base::Time valid_until) {
+  // TODO(maksymb): Move to separate class.
+
   if (base::Time::Now() > valid_until) {
     reg_info_.confirmation_state = RegistrationInfo::CONFIRMATION_TIMEOUT;
     LOG(INFO) << "Confirmation timeout reached.";
@@ -723,9 +754,28 @@ void Printer::PostOnIdle() {
       base::Bind(&Printer::OnIdle, AsWeakPtr()));
 }
 
+void Printer::CheckRegistrationExpiration() {
+  if (!registration_expiration_.is_null() &&
+      registration_expiration_ < base::Time::Now()) {
+    reg_info_ = RegistrationInfo();
+    InvalidateRegistrationExpiration();
+    if (connection_state_ != ONLINE)
+      TryConnect();
+  }
+}
+
+void Printer::UpdateRegistrationExpiration() {
+  registration_expiration_ =
+      base::Time::Now() + base::TimeDelta::FromSeconds(kRegistrationTimeout);
+}
+
+void Printer::InvalidateRegistrationExpiration() {
+  registration_expiration_ = base::Time();
+}
+
 PrivetHttpServer::RegistrationErrorStatus
-    Printer::ConfirmationToRegistrationError(
-        RegistrationInfo::ConfirmationState state) {
+Printer::ConfirmationToRegistrationError(
+    RegistrationInfo::ConfirmationState state) {
   switch (state) {
     case RegistrationInfo::CONFIRMATION_PENDING:
       return PrivetHttpServer::REG_ERROR_PENDING_USER_ACTION;
@@ -759,6 +809,27 @@ std::string Printer::ConnectionStateToString(ConnectionState state) const {
   }
 }
 
+void Printer::FallOffline(bool instant_reconnect) {

[Vitaly Buka (NO REVIEWS), 2013/08/15 17:48:06]

I guess you can remove argument and use TryConnect() where FallOffline(true) is
used

[maksymb, 2013/08/15 18:39:13]

1. In this case ChangeState(OFFLINE) won't be called and   |requester_| and
|xmpp_listener_| won't be reset.
2. Also, I want to block registration and set ERROR if I receive server or
network error.

[Vitaly Buka (NO REVIEWS), 2013/08/15 20:17:06]

It won't be called anyway
DCHECK is debug only.

On 2013/08/15 18:39:13, maksymb wrote:

[maksymb, 2013/08/15 20:23:00]

Moved outside of DCHECK.

+  DCHECK(ChangeState(OFFLINE));
+
+  if (!IsRegistered()) {
+    SetRegistrationError("Cannot access server during registration process");
+    TryConnect();  // For registration errors reconnection is instant, because
+                   // we don't need to do anything when Printer will become
+                   // connected: we will be waiting for new HTTP requests.
+    return;
+  }
+
+  if (instant_reconnect) {
+    TryConnect();
+  } else {
+    base::MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&Printer::TryConnect, AsWeakPtr()),
+        base::TimeDelta::FromSeconds(kReconnectTimeout));

[Vitaly Buka (NO REVIEWS), 2013/08/15 17:48:06]

Fix alignment

+  }
+}
+
 bool Printer::ChangeState(ConnectionState new_state) {
   if (connection_state_ == new_state)
     return false;
@@ -771,26 +842,9 @@ bool Printer::ChangeState(ConnectionState new_state) {
 
   dns_server_.UpdateMetadata(CreateTxt());
 
-  switch (connection_state_) {
-    case CONNECTING:
-      break;
-
-    case ONLINE:
-      break;
-
-    case OFFLINE:
-      requester_.reset();
-      xmpp_listener_.reset();
-      base::MessageLoop::current()->PostDelayedTask(
-          FROM_HERE,
-          base::Bind(&Printer::TryConnect, AsWeakPtr()),
-          base::TimeDelta::FromSeconds(kReconnectTimeout));
-
-    case NOT_CONFIGURED:
-      break;
-
-    default:
-      NOTREACHED();
+  if (connection_state_ == OFFLINE) {
+    requester_.reset();
+    xmpp_listener_.reset();
   }
 
   return true;