arc: Provide progress to load TOS content for OptIn.

chrome/browser/resources/chromeos/arc_support/background.js

Index: chrome/browser/resources/chromeos/arc_support/background.js
diff --git a/chrome/browser/resources/chromeos/arc_support/background.js b/chrome/browser/resources/chromeos/arc_support/background.js
index d72b57df6ae25f91600c1f021d5aad971b0da6f7..4ab0052356434223af53b162c1cbbd339cf2fddd 100644
--- a/chrome/browser/resources/chromeos/arc_support/background.js
+++ b/chrome/browser/resources/chromeos/arc_support/background.js
@@ -8,7 +8,8 @@
  * @type {Array<string>}
  */
 var UI_PAGES = ['none',
-                'start',
+                'terms-loading',
+                'terms',
                 'lso-loading',
                 'lso',
                 'arc-loading',
@@ -59,6 +60,12 @@ var termsReloadTimeout = null;
 var currentDeviceId = null;
 
 /**
+ * Indicates that terms were accepted by user.
+ * @type {boolean}
+ */
+var termsAccepted = false;
+
+/**
  * Closes current window in response to request from native code. This does not
  * issue 'cancelAuthCode' message to native code.
  */
@@ -105,9 +112,6 @@ function initialize(data, deviceId) {
         js: { files: ['playstore.js'] },
         run_at: 'document_end'
       }]);
-
-  // Applying localization changes page layout, update terms height.
-  updateTermsHeight();

[khmel, 2016/07/08 21:59:33]

No longer needed here, we update height when activating 'terms' page.

[xiyuan, 2016/07/08 22:33:29]

Acknowledged.

 }
 
 /**
@@ -171,6 +175,7 @@ function updateTermsHeight() {
   var setTermsHeight = function() {
     var doc = appWindow.contentWindow.document;
     var termsContainer = doc.getElementById('terms-container');
+    termsView.style.height = '0px';

[khmel, 2016/07/08 21:59:33]

For some reason getComputedStyle depends on webview height but based on html
layout (terms-container is marked as flex auto) should not. This causes
non-stable computation (each next call gives different result). As result,
layout is noticeable changed on next update and 'agree'/'cancel' buttons are
distorted. Having 0px height before computation stabilize layout.

[xiyuan, 2016/07/08 22:33:29]

Strange since we have "height: 100%" on webview tag [1] and it should auto
resized with its parent container. Maybe adding "position: relative" to
#terms-container would get rid of this manual setting height code.

I probably have proposed this before. But anyway, in case it does not work, I am
fine with this work around. Just add a comment to doc why we are doing this.

[1]:
https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/arc_su...

[khmel, 2016/07/08 22:52:55]

Was good try but did not help :(. Added comment.

     var style = window.getComputedStyle(termsContainer, null);
     var height = style.getPropertyValue('height');
     termsView.style.height = height;
@@ -277,10 +282,17 @@ function showPageWithStatus(pageId, status) {
     return;
   }
 
-  if (UI_PAGES[pageId] == 'start') {
+  if (UI_PAGES[pageId] == 'terms-loading') {
+    termsAccepted = false;
     loadInitialTerms();
-  } else if (UI_PAGES[pageId] == 'error' ||
-             UI_PAGES[pageId] == 'error-with-feedback') {
+  } else {
+    // Explicit request to start not from start page. Assume terms are
+    // accepted in this case.
+    termsAccepted = true;
+  }
+
+  if (UI_PAGES[pageId] == 'error' ||
+      UI_PAGES[pageId] == 'error-with-feedback') {
     setErrorMessage(status);
   }
   showPage(UI_PAGES[pageId]);
@@ -297,25 +309,32 @@ chrome.app.runtime.onLaunched.addListener(function() {
   var onAppContentLoad = function() {
     var doc = appWindow.contentWindow.document;
     lsoView = doc.getElementById('arc-support');
-    // Apply absolute dimension to webview tag in order to avoid UI glitch

[khmel, 2016/07/08 21:59:33]

You might remember that this was  done to avoid weird webview animation on LSO
load. Before we fixed it by setting fixed size as inner window bounds. Recently
image header was also added to this page and this calculation is no longer true.
As variant it is possible to set fixed width in css which fixes this as well.

[xiyuan, 2016/07/08 22:33:29]

Acknowledged.

-    // when embedded content layout is visible for user, even if 100% width and
-    // height are set in css file.
-    // TODO(khmel): Investigate why relative layout is not enough.
-    lsoView.style.width = appWindow.innerBounds.width + 'px';
-    lsoView.style.height = appWindow.innerBounds.height + 'px';
 
     var isApprovalResponse = function(url) {
       var resultUrlPrefix = 'https://accounts.google.com/o/oauth2/approval?';
       return url.substring(0, resultUrlPrefix.length) == resultUrlPrefix;
     };
 
+    var lsoError = false;

[khmel, 2016/07/08 21:59:33]

Initially I was trying to call webview.stop() method on error in order to
prevent calling onLsoViewContentLoad. However in this case, webview is no longer
responsible afterword (when I try to assign new src, ERR_ABORTED is always
returned and webview.reload() does not help).

[xiyuan, 2016/07/08 22:33:29]

Acknowledged.

     var onLsoViewRequestResponseStarted = function(details) {
       if (isApprovalResponse(details.url)) {
         showPage('arc-loading');
       }
+      lsoError = false;
+    };
+
+    var onLsoViewErrorOccurred = function(details) {
+      setErrorMessage(appWindow.contentWindow.loadTimeData.getString(
+          'serverError'));
+      showPage('error');
+      lsoError = true;
     };
 
     var onLsoViewContentLoad = function() {
+      if (lsoError) {
+        return;
+      }
+
       if (!isApprovalResponse(lsoView.src)) {
         // Show LSO page when its content is ready.
         showPage('lso');
@@ -324,7 +343,8 @@ chrome.app.runtime.onLaunched.addListener(function() {
 
       lsoView.executeScript({code: 'document.title;'}, function(results) {
         var authCodePrefix = 'Success code=';
-        if (results[0].substring(0, authCodePrefix.length) ==
+        if (results[0].length &&

[khmel, 2016/07/08 21:59:33]

Before error was not handled and results[0] generated error and stopping js
processing. This caused situation when technical info might be shown to user in
case of some error.

[xiyuan, 2016/07/08 22:33:29]

Should this be 
  results && results.length == 1 && typeof results[0] == 'string'
?

Otherwise, we still hits error when results is null, [] or non string in array.

[khmel, 2016/07/08 22:52:55]

This is much better. Honestly speaking results[0].length was incorrect and I
forgot to update the code but you proposal is safer.

+            results[0].substring(0, authCodePrefix.length) ==
             authCodePrefix) {
           var authCode = results[0].substring(authCodePrefix.length);
           sendNativeMessage('setAuthCode', {code: authCode});
@@ -343,31 +363,40 @@ chrome.app.runtime.onLaunched.addListener(function() {
 
     lsoView.request.onResponseStarted.addListener(
         onLsoViewRequestResponseStarted, requestFilter);
+    lsoView.request.onErrorOccurred.addListener(
+        onLsoViewErrorOccurred, requestFilter);
     lsoView.addEventListener('contentload', onLsoViewContentLoad);
 
-    termsView = doc.getElementById('terms');
+    termsView = doc.getElementById('terms-view');
 
-    // Handle terms view completed event. Enable button 'Agree' in case terms
-    // were loaded successfully and try to reload its content on error.
-    var termsReloadRetryTimeMs = 1000;  // 1 second
-    function onTermsViewRequestCompleted(details) {
-      if (termsReloadTimeout) {
-        clearTimeout(termsReloadTimeout);
-        termsReloadTimeout = null;
-      }
-      if (details.statusCode == 200) {
-        doc.getElementById('button-agree').disabled = false;
-      } else {
-        termsReloadTimeout = setTimeout(loadInitialTerms,
-                                        termsReloadRetryTimeMs);
-        termsReloadRetryTimeMs = termsReloadRetryTimeMs * 2;
-        if (termsReloadRetryTimeMs > 30000) {
-          termsReloadRetryTimeMs = 30000;
-        }
+    var termsError = false;
+    var onTermsViewBeforeRequest = function(details) {
+      showPage('terms-loading');
+      termsError = false;
+    };
+
+    var onTermsViewErrorOccurred = function(details) {
+      termsAccepted = false;
+      setErrorMessage(appWindow.contentWindow.loadTimeData.getString(
+          'serverError'));
+      showPage('error');
+      termsError = true;
+    };
+
+    var onTermsViewContentLoad = function() {
+      if (termsError) {
+        return;
       }
-    }
-    termsView.request.onCompleted.addListener(onTermsViewRequestCompleted,
-                                              requestFilter);
+      showPage('terms');
+      updateTermsHeight();
+    };
+
+    termsView.request.onBeforeRequest.addListener(onTermsViewBeforeRequest,
+                                                  requestFilter);
+    termsView.request.onErrorOccurred.addListener(onTermsViewErrorOccurred,
+                                                  requestFilter);
+    termsView.addEventListener('contentload', onTermsViewContentLoad);
+
 
     // webview is not allowed to open links in the new window. Hook these events
     // and open links in context of main page.
@@ -377,6 +406,8 @@ chrome.app.runtime.onLaunched.addListener(function() {
     });
 
     var onAgree = function() {
+      termsAccepted = true;
+
       var enableMetrics = doc.getElementById('enable-metrics');
       if (!enableMetrics.hidden) {
         sendNativeMessage('enableMetrics', {
@@ -401,7 +432,11 @@ chrome.app.runtime.onLaunched.addListener(function() {
     };
 
     var onRetry = function() {
-      sendNativeMessage('startLso');
+      if (termsAccepted) {
+        sendNativeMessage('startLso');
+      } else {
+        loadInitialTerms();
+      }
     };
 
     var onSendFeedback = function() {