DRTVWR-474: Make login coroutine sync with updater process on failure.

Specifically, introduce an LLEventMailDrop("LoginSync"). When the updater
detects that an update is required, it will post to that rendezvous point.

When login.cgi responds with login failure, make the login coroutine wait (a
few seconds) for that ping from the updater.

If we receive that ping and if it contains a "reply" key, make the fail.login
listener respond to the updater with an indication of whether to proceed with
update.

If both login.cgi and the updater concur that an update is required, produce a
new confirmation message for the user and then (once user responds) tell the
updater to proceed. Otherwise, produce the usual login-failure message and
tell the updater never mind.

Introduce LLCoro::OverrideConsuming to provide temporary save/restore of the
set_consuming() / get_consuming() flag. It's a good idea to set the consuming
flag when retrieving data from an LLEventMailDrop.

5 files changed, 142 insertions, 16 deletions

diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h
index 8fb27af6a4..c551413811 100644
--- a/indra/llcommon/llcoros.h
+++ b/indra/llcommon/llcoros.h
@@ -170,6 +170,26 @@ public:
     static bool get_consuming();
 
     /**
+     * RAII control of the consuming flag
+     */
+    class OverrideConsuming
+    {
+    public:
+        OverrideConsuming(bool consuming):
+            mPrevConsuming(get_consuming())
+        {
+            set_consuming(consuming);
+        }
+        ~OverrideConsuming()
+        {
+            set_consuming(mPrevConsuming);
+        }
+
+    private:
+        bool mPrevConsuming;
+    };
+
+    /**
      * Please do NOT directly use boost::dcoroutines::future! It is essential
      * to maintain the "current" coroutine at every context switch. This
      * Future wraps the essential boost::dcoroutines::future functionality
diff --git a/indra/newview/lllogininstance.cpp b/indra/newview/lllogininstance.cpp
index bc93fa2c20..126c6be388 100644
--- a/indra/newview/lllogininstance.cpp
+++ b/indra/newview/lllogininstance.cpp
@@ -253,14 +253,12 @@ bool LLLoginInstance::handleLoginEvent(const LLSD& event)
 
 	mLoginState = event["state"].asString();
 	mResponseData = event["data"];
-	
+
 	if(event.has("transfer_rate"))
 	{
 		mTransferRate = event["transfer_rate"].asReal();
 	}
 
-	
-
 	// Call the method registered in constructor, if any, for more specific
 	// handling
 	mDispatcher.try_call(event);
@@ -276,6 +274,14 @@ void LLLoginInstance::handleLoginFailure(const LLSD& event)
     // Login has failed. 
     // Figure out why and respond...
     LLSD response = event["data"];
+    LLSD updater  = response["updater"];
+
+    // Always provide a response to the updater, if in fact the updater
+    // contacted us, if in fact the ping contains a 'reply' key. Most code
+    // paths tell it not to proceed with updating.
+    ResponsePtr resp(std::make_shared<LLEventAPI::Response>
+                         (LLSDMap("update", false), updater));
+
     std::string reason_response = response["reason"].asString();
     std::string message_response = response["message"].asString();
     LL_DEBUGS("LLLogin") << "reason " << reason_response
@@ -328,17 +334,44 @@ void LLLoginInstance::handleLoginFailure(const LLSD& event)
     }
     else if(reason_response == "update")
     {
-        // This shouldn't happen - the viewer manager should have forced an update; 
-        // possibly the user ran the viewer directly and bypassed the update check
+        // This can happen if the user clicked Login quickly, before we heard
+        // back from the Viewer Version Manager, but login failed because
+        // login.cgi is insisting on a required update. We were called with an
+        // event that bundles both the login.cgi 'response' and the
+        // synchronization event from the 'updater'.
         std::string required_version = response["message_args"]["VERSION"];
         LL_WARNS("LLLogin") << "Login failed because an update to version " << required_version << " is required." << LL_ENDL;
 
         if (gViewerWindow)
             gViewerWindow->setShowProgress(FALSE);
 
-        LLSD data(LLSD::emptyMap());
-        data["VERSION"] = required_version;
-        LLNotificationsUtil::add("RequiredUpdate", data, LLSD::emptyMap(), boost::bind(&LLLoginInstance::handleLoginDisallowed, this, _1, _2));
+        LLSD args(LLSDMap("VERSION", required_version));
+        if (updater.isUndefined())
+        {
+            // If the updater failed to shake hands, better advise the user to
+            // download the update him/herself.
+            LLNotificationsUtil::add(
+                "RequiredUpdate",
+                args,
+                updater,
+                boost::bind(&LLLoginInstance::handleLoginDisallowed, this, _1, _2));
+        }
+        else
+        {
+            // If we've heard from the updater that an update is required,
+            // then display the prompt that assures the user we'll take care
+            // of it. This is the one case in which we bind 'resp':
+            // instead of destroying our Response object (and thus sending a
+            // negative reply to the updater) as soon as we exit this
+            // function, bind our shared_ptr so it gets passed into
+            // syncWithUpdater. That ensures that the response is delayed
+            // until the user has responded to the notification.
+            LLNotificationsUtil::add(
+                "PauseForUpdate",
+                args,
+                updater,
+                boost::bind(&LLLoginInstance::syncWithUpdater, this, resp, _1, _2));
+        }
     }
     else if(   reason_response == "key"
             || reason_response == "presence"
@@ -361,6 +394,19 @@ void LLLoginInstance::handleLoginFailure(const LLSD& event)
     }   
 }
 
+void LLLoginInstance::syncWithUpdater(ResponsePtr resp, const LLSD& notification, const LLSD& response)
+{
+    LL_INFOS("LLLogin") << "LLLoginInstance::syncWithUpdater" << LL_ENDL;
+    // 'resp' points to an instance of LLEventAPI::Response that will be
+    // destroyed as soon as we return and the notification response functor is
+    // unregistered. Modify it so that it tells the updater to go ahead and
+    // perform the update. Naturally, if we allowed the user a choice as to
+    // whether to proceed or not, this assignment would reflect the user's
+    // selection.
+    (*resp)["update"] = true;
+    attemptComplete();
+}
+
 void LLLoginInstance::handleLoginDisallowed(const LLSD& notification, const LLSD& response)
 {
     attemptComplete();
@@ -420,7 +466,6 @@ bool LLLoginInstance::handleTOSResponse(bool accepted, const std::string& key)
 	return true;
 }
 
-
 std::string construct_start_string()
 {
 	std::string start;
diff --git a/indra/newview/lllogininstance.h b/indra/newview/lllogininstance.h
index 651ad10afb..b759b43474 100644
--- a/indra/newview/lllogininstance.h
+++ b/indra/newview/lllogininstance.h
@@ -28,8 +28,10 @@
 #define LL_LLLOGININSTANCE_H
 
 #include "lleventdispatcher.h"
+#include "lleventapi.h"
 #include <boost/scoped_ptr.hpp>
 #include <boost/function.hpp>
+#include <memory>                   // std::shared_ptr
 #include "llsecapi.h"
 class LLLogin;
 class LLEventStream;
@@ -68,6 +70,7 @@ public:
 	LLNotificationsInterface& getNotificationsInterface() const { return *mNotifications; }
 
 private:
+	typedef std::shared_ptr<LLEventAPI::Response> ResponsePtr;
 	void constructAuthParams(LLPointer<LLCredential> user_credentials);
 	void updateApp(bool mandatory, const std::string& message);
 	bool updateDialogCallback(const LLSD& notification, const LLSD& response);
@@ -77,7 +80,8 @@ private:
 	void handleLoginSuccess(const LLSD& event);
 	void handleDisconnect(const LLSD& event);
 	void handleIndeterminate(const LLSD& event);
-    void handleLoginDisallowed(const LLSD& notification, const LLSD& response);
+	void handleLoginDisallowed(const LLSD& notification, const LLSD& response);
+	void syncWithUpdater(ResponsePtr resp, const LLSD& notification, const LLSD& response);
 
 	bool handleTOSResponse(bool v, const std::string& key);
 
diff --git a/indra/newview/skins/default/xui/en/notifications.xml b/indra/newview/skins/default/xui/en/notifications.xml
index 9eaa5330c3..5f9b8adc89 100644
--- a/indra/newview/skins/default/xui/en/notifications.xml
+++ b/indra/newview/skins/default/xui/en/notifications.xml
@@ -3841,7 +3841,6 @@ Finished download of raw terrain file to:
    name="RequiredUpdate"
    type="alertmodal">
 Version [VERSION] is required for login.
-This should have been updated for you but apparently was not.
 Please download from https://secondlife.com/support/downloads/
     <tag>confirm</tag>
     <usetemplate
@@ -3851,6 +3850,18 @@ Please download from https://secondlife.com/support/downloads/
 
   <notification
    icon="alertmodal.tga"
+   name="PauseForUpdate"
+   type="alertmodal">
+Version [VERSION] is required for login.
+Click OK to download and install.
+    <tag>confirm</tag>
+    <usetemplate
+     name="okbutton"
+     yestext="OK"/>
+  </notification>
+
+  <notification
+   icon="alertmodal.tga"
    name="LoginFailedUnknown"
    type="alertmodal">
 Sorry, login failed for an unrecognized reason.
diff --git a/indra/viewer_components/login/lllogin.cpp b/indra/viewer_components/login/lllogin.cpp
index c767d52c7b..9193d32b49 100644
--- a/indra/viewer_components/login/lllogin.cpp
+++ b/indra/viewer_components/login/lllogin.cpp
@@ -128,6 +128,15 @@ void LLLogin::Impl::connect(const std::string& uri, const LLSD& login_params)
     LL_DEBUGS("LLLogin") << " connected with  uri '" << uri << "', login_params " << login_params << LL_ENDL;	
 }
 
+namespace {
+// Instantiate this rendezvous point at namespace scope so it's already
+// present no matter how early the updater might post to it.
+// Use an LLEventMailDrop, which has future-like semantics: regardless of the
+// relative order in which post() or listen() are called, it delivers each
+// post() event to its listener(s) until one of them consumes that event.
+static LLEventMailDrop sSyncPoint("LoginSync");
+}
+
 void LLLogin::Impl::loginCoro(std::string uri, LLSD login_params)
 {
     LLSD printable_params = login_params;
@@ -219,7 +228,44 @@ void LLLogin::Impl::loginCoro(std::string uri, LLSD login_params)
         }
         else
         {
-            sendProgressEvent("offline", "fail.login", mAuthResponse["responses"]);
+            // Synchronize here with the updater. We synchronize here rather
+            // than in the fail.login handler, which actually examines the
+            // response from login.cgi, because here we are definitely in a
+            // coroutine and can definitely use suspendUntilBlah(). Whoever's
+            // listening for fail.login might not be.
+
+            // If the reason for login failure is that we must install a
+            // required update, we definitely want to pass control to the
+            // updater to manage that for us. We'll handle any other login
+            // failure ourselves, as usual. We figure that no matter where you
+            // are in the world, or what kind of network you're on, we can
+            // reasonably expect the Viewer Version Manager to respond more or
+            // less as quickly as login.cgi. This synchronization is only
+            // intended to smooth out minor races between the two services.
+            // But what if the updater crashes? Use a timeout so that
+            // eventually we'll tire of waiting for it and carry on as usual.
+            // Given the above, it can be a fairly short timeout, at least
+            // from a human point of view.
+
+            // Since sSyncPoint is an LLEventMailDrop, we DEFINITELY want to
+            // consume the posted event.
+            LLCoros::OverrideConsuming oc(true);
+            // Timeout should produce the isUndefined() object passed here.
+            LL_DEBUGS("LLLogin") << "Login failure, waiting for sync from updater" << LL_ENDL;
+            LLSD updater = llcoro::suspendUntilEventOnWithTimeout(sSyncPoint, 10, LLSD());
+            if (updater.isUndefined())
+            {
+                LL_WARNS("LLLogin") << "Failed to hear from updater, proceeding with fail.login"
+                                    << LL_ENDL;
+            }
+            else
+            {
+                LL_DEBUGS("LLLogin") << "Got responses from updater and login.cgi" << LL_ENDL;
+            }
+            // Let the fail.login handler deal with empty updater response.
+            LLSD responses(mAuthResponse["responses"]);
+            responses["updater"] = updater;
+            sendProgressEvent("offline", "fail.login", responses);
         }
         return;             // Done!
     }
@@ -249,10 +295,10 @@ void LLLogin::Impl::loginCoro(std::string uri, LLSD login_params)
     // *NOTE: The response from LLXMLRPCListener's Poller::poll method returns an
     // llsd with no "responses" node. To make the output from an incomplete login symmetrical 
     // to success, add a data/message and data/reason fields.
-    LLSD error_response;
-    error_response["reason"] = mAuthResponse["status"];
-    error_response["errorcode"] = mAuthResponse["errorcode"];
-    error_response["message"] = mAuthResponse["error"];
+    LLSD error_response(LLSDMap
+                        ("reason",    mAuthResponse["status"])
+                        ("errorcode", mAuthResponse["errorcode"])
+                        ("message",   mAuthResponse["error"]));
     if(mAuthResponse.has("certificate"))
     {
         error_response["certificate"] = mAuthResponse["certificate"];