Fix for EXT-6663 (TOS Screen logic for enabling agreement checkbox is questionable)

* remove the use of the start_url parameter, and instead store the loading url as a string in the xui, so that there's no chance of the loading screen page completing before we add our observer.
* replace the "load complete count" with explicit state variables for whether the loading screen has completed and whether the site is alive
* don't begin trying to load the real URL until both of the above conditions are met (and begin the load no matter which order they are satisfied in)

Reviewed by Richard at http://codereview.lindenlab.com/1218007

3 files changed, 47 insertions, 9 deletions

diff --git a/indra/newview/llfloatertos.cpp b/indra/newview/llfloatertos.cpp
index 69ee8cd547..3db9587797 100644
--- a/indra/newview/llfloatertos.cpp
+++ b/indra/newview/llfloatertos.cpp
@@ -57,7 +57,9 @@ LLFloaterTOS::LLFloaterTOS(const LLSD& data)
 :	LLModalDialog( data["message"].asString() ),
 	mMessage(data["message"].asString()),
 	mWebBrowserWindowId( 0 ),
-	mLoadCompleteCount( 0 ),
+	mLoadingScreenLoaded(false),
+	mSiteAlive(false),
+	mRealNavigateBegun(false),
 	mReplyPumpName(data["reply_pump"].asString())
 {
 }
@@ -138,6 +140,11 @@ BOOL LLFloaterTOS::postBuild()
 	if ( web_browser )
 	{
 		web_browser->addObserver(this);
+
+		// Don't use the start_url parameter for this browser instance -- it may finish loading before we get to add our observer.
+		// Store the URL separately and navigate here instead.
+		web_browser->navigateTo( getString( "loading_url" ) );
+		
 		gResponsePtr = LLIamHere::build( this );
 		LLHTTPClient::get( getString( "real_url" ), gResponsePtr );
 	}
@@ -147,15 +154,16 @@ BOOL LLFloaterTOS::postBuild()
 
 void LLFloaterTOS::setSiteIsAlive( bool alive )
 {
+	mSiteAlive = alive;
+	
 	// only do this for TOS pages
 	if (hasChild("tos_html"))
 	{
-		LLMediaCtrl* web_browser = getChild<LLMediaCtrl>("tos_html");
 		// if the contents of the site was retrieved
 		if ( alive )
 		{
 			// navigate to the "real" page 
-			web_browser->navigateTo( getString( "real_url" ) );
+			loadIfNeeded();
 		}
 		else
 		{
@@ -167,6 +175,19 @@ void LLFloaterTOS::setSiteIsAlive( bool alive )
 	}
 }
 
+void LLFloaterTOS::loadIfNeeded()
+{
+	if(!mRealNavigateBegun && mSiteAlive)
+	{
+		LLMediaCtrl* web_browser = getChild<LLMediaCtrl>("tos_html");
+		if(web_browser)
+		{
+			mRealNavigateBegun = true;
+			web_browser->navigateTo( getString( "real_url" ) );
+		}
+	}
+}
+
 LLFloaterTOS::~LLFloaterTOS()
 {
 
@@ -216,8 +237,13 @@ void LLFloaterTOS::onCancel( void* userdata )
 		LLEventPumps::instance().obtain(self->mReplyPumpName).post(LLSD(false));
 	}
 
-	self->mLoadCompleteCount = 0;  // reset counter for next time we come to TOS
-	self->closeFloater(); // destroys this object
+	// reset state for next time we come to TOS
+	self->mLoadingScreenLoaded = false;
+	self->mSiteAlive = false;
+	self->mRealNavigateBegun = false;
+	
+	// destroys this object
+	self->closeFloater(); 
 }
 
 //virtual 
@@ -225,8 +251,12 @@ void LLFloaterTOS::handleMediaEvent(LLPluginClassMedia* /*self*/, EMediaEvent ev
 {
 	if(event == MEDIA_EVENT_NAVIGATE_COMPLETE)
 	{
-		// skip past the loading screen navigate complete
-		if ( ++mLoadCompleteCount == 2 )
+		if(!mLoadingScreenLoaded)
+		{
+			mLoadingScreenLoaded = true;
+			loadIfNeeded();
+		}
+		else if(mRealNavigateBegun)
 		{
 			llinfos << "NAVIGATE COMPLETE" << llendl;
 			// enable Agree to TOS radio button now that page has loaded
diff --git a/indra/newview/llfloatertos.h b/indra/newview/llfloatertos.h
index 1d573e8170..6ea56408ee 100644
--- a/indra/newview/llfloatertos.h
+++ b/indra/newview/llfloatertos.h
@@ -66,9 +66,14 @@ public:
 	/*virtual*/ void handleMediaEvent(LLPluginClassMedia* self, EMediaEvent event);
 
 private:
+
+	void			loadIfNeeded();
+	
 	std::string		mMessage;
 	int				mWebBrowserWindowId;
-	int				mLoadCompleteCount;
+	bool			mLoadingScreenLoaded;
+	bool			mSiteAlive;
+	bool			mRealNavigateBegun;
 	std::string		mReplyPumpName;
 };
 
diff --git a/indra/newview/skins/default/xui/en/floater_tos.xml b/indra/newview/skins/default/xui/en/floater_tos.xml
index f3665e87ed..cbfaac958b 100644
--- a/indra/newview/skins/default/xui/en/floater_tos.xml
+++ b/indra/newview/skins/default/xui/en/floater_tos.xml
@@ -11,6 +11,10 @@
      name="real_url">
         http://secondlife.com/app/tos/
     </floater.string>
+    <floater.string
+     name="loading_url">
+        data:text/html,%3Chtml%3E%3Chead%3E%3C/head%3E%3Cbody text=%22000000%22%3E%3Ch2%3E Loading %3Ca%20target%3D%22_external%22%20href%3D%22http%3A//secondlife.com/app/tos/%22%3ETerms%20of%20Service%3C/a%3E...%3C/h2%3E %3C/body%3E %3C/html%3E
+    </floater.string>
     <button
      enabled="false"
      height="20"
@@ -59,7 +63,6 @@
      layout="topleft"
      left_delta="0"
      name="tos_html"
-     start_url="data:text/html,%3Chtml%3E%3Chead%3E%3C/head%3E%3Cbody text=%22000000%22%3E%3Ch2%3E Loading %3Ca%20target%3D%22_external%22%20href%3D%22http%3A//secondlife.com/app/tos/%22%3ETerms%20of%20Service%3C/a%3E...%3C/h2%3E %3C/body%3E %3C/html%3E"
      top_delta="40"
      width="568" />
 </floater>