Handle host need upgrade in Pin browser test

remoting/webapp/browser_test/browser_test.js

Index: remoting/webapp/browser_test/browser_test.js
diff --git a/remoting/webapp/browser_test/browser_test.js b/remoting/webapp/browser_test/browser_test.js
index 786000291a468554220106784aefca1e846718cd..2c385bb986a59c9035cde3fea17674bd06313b0f 100644
--- a/remoting/webapp/browser_test/browser_test.js
+++ b/remoting/webapp/browser_test/browser_test.js
@@ -156,6 +156,21 @@ browserTest.onUIMode = function(expectedMode, opt_timeout) {
   });
 };
 
+browserTest.connectMe2Me = function() {
+  var AppMode = remoting.AppMode;
+  browserTest.clickOnControl('this-host-connect');
+  return browserTest.onUIMode(AppMode.CLIENT_HOST_NEEDS_UPGRADE).then(
+    function() {
+      // On fulfilled.
+      browserTest.clickOnControl('host-needs-update-connect-button');

[Jamie, 2014/07/11 19:33:22]

Rather than silently dismissing the dialog if it appears, would it be better to
pass in whether or not it's expected? We should have a test for this feature,
not treat the two cases the same. If it's convenient to do that in the short
term to unblock testing then that's okay, but there should at least be a TODO.

[kelvinp, 2014/07/11 21:11:22]

Good question. Browser tests are configured in the lab to run with various
combinations of the web app against the host.  For instance, Anand is planning
to set up a rolling build bot that runs the web app in the daily build against
the released host in Lorry, which will always "need upgrade"

In my opinion, most test should be agnostic to whether the host needs upgrade or
not. For instance, if the pin can be upgraded correctly, the pin upgrade test
should pass even if the host need upgrade dialog shows up.

Therefore, I don't want to expose an extra flag in the function to fail the test
when the host need upgrade dialog shows up as it may make our test flaky.  In
fact, the C++ browser tests has exactly the same behavior.
RemoteDesktopBrowserTest::ConnectToLocalHost silently dismisses the upgrade
dialog under the hood.

The only exception would be the "host needs upgrade test" suggested in your
comment.  In that case, we can write a separate browser case to verify the
behavior.
What do you think?
 

[Jamie, 2014/07/14 18:24:04]

That was pretty much what I had in mind. I think you're right that we don't want
every test to need to know about this dialog, but we'll probably want to re-use
this most of this code when we write one that does. I was thinking along the
lines of an enum { OPTIONAL, REQUIRED, DISALLOWED } for the upgrade dialog, and
make it default to OPTIONAL. It doesn't need to be this CL though.

+    }, function() {
+      // On time out.
+      return Promise.resolve();
+    }).then(function() {
+      return browserTest.onUIMode(AppMode.CLIENT_PIN_PROMPT);
+    });
+}
+
 browserTest.expectMe2MeError = function(errorTag) {
   var AppMode = remoting.AppMode;
   var Timeout = browserTest.Timeout;