Fix losing selected language on ToS page reloading.

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * UI Pages. Note the order must be in sync with the ArcAuthService::UIPage
  * enum.
  * @type {Array<string>}
  */
 var UI_PAGES = ['none',
@@ skipping 466 matching lines @@
   if (!appWindow) {
     return;
   }
 
   if (UI_PAGES[pageId] == 'terms-loading') {
     termsAccepted = arcManaged;
     if (termsAccepted) {
       showPage('terms');
       return;
     }
-    loadInitialTerms();
+    loadTerms();
   } 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]);
 }
 
 /**
- * Loads initial Play Store terms.
+ * Loads Play Store terms.
  */
-function loadInitialTerms() {
-  termsView.src = 'https://play.google.com/about/play-terms.html';
+function loadTerms() {
+  if (termsView.src) {

[khmel, 2016/10/26 20:35:20]

This might be dangerous. Assume ToS decides to redirect to some page, which is
different from what we set during the page processing and user selection. This
can lead to situation when ToS gets stuck on this page. That is why full reload
was implemented which is safer. Could you please make sure that src URL looks at
least as ToS target.

[hidehiko, 2016/10/27 04:01:48]

I don't think the old code handles the case, because there is no explicit way
for a user to show RETRY button intentionally. Practically, the easiest way, I
think, is closing the Chrome App and re-enable ARC, so that the Chrome App is
shown again. The behavior is still preserved, even with this CL.

Also, if ToS page navigates a user to another page, it should be considered
still as a part of ToS? I think it's ToS's responsibility, shouldn't it?

[khmel, 2016/10/27 14:40:57]

Sorry, don't get what do you mean? Old code has simple logic. Navigate to
default target, select option based on current combination of lang and location
and load terms for it. In case of error it repeats this step.
It is potentially possible to change several pages when coming to final user
selection. My concern is before reloading this particular page let make sure
that it looks like https://play.google.com/intl/.../about/play-terms.html.
Otherwise you may continue reloading wrong page, for example some
https://google.com/not_allowed, wrong_dns or any other not related to ToS

[hidehiko, 2016/10/27 16:41:01]

Sorry, I still am confused.

"ToS decides to redirect to some page, which is
different from what we set during the page processing and user selection."

So the user see the redirected webpage, on ToS page. So, the user won't see the
RETRY button???
I also didn't get understand your point yet...

How users are navigated to such a page?
If it is linked from ToS webpage, then should we think it is a part of ToS?

Or, if what you said is "We should not show the page other than
play.google.com/intl/${lang}/about/play-terms.html in any cases", then this
function is not the place to handle it, IIUC, considering redirection.

+    // This is reloading the page, typically clicked RETRY on error page.
+    termsView.reload();
+  } else {
+    // This is first loading case so set the URL explicitly.
+    termsView.src = 'https://play.google.com/about/play-terms.html';
+  }
 }
 
 function setWindowBounds() {
   if (!appWindow) {
     return;
   }
 
   var decorationWidth = appWindow.outerBounds.width -
       appWindow.innerBounds.width;
   var decorationHeight = appWindow.outerBounds.height -
@@ skipping 142 matching lines @@
         appWindow.close();
       }
     };
 
     var onRetry = function() {
       if (termsAccepted) {
         // Reuse the onAgree() in case that the user has already accepted
         // the ToS.
         onAgree();
       } else {
-        loadInitialTerms();
+        loadTerms();

[hidehiko, 2016/10/26 13:33:54]

Note: Typically, this is the reload case. However, the error page can be shown
"before" the ToS page, so this could be first loading, theoretically... That's
why we share the loadTerms().

       }
     };
 
     var onSendFeedback = function() {
       sendNativeMessage('onSendFeedbackClicked');
     };
 
     doc.getElementById('button-agree').addEventListener('click', onAgree);
     doc.getElementById('button-cancel').addEventListener('click', onCancel);
     doc.getElementById('button-retry').addEventListener('click', onRetry);
@@ skipping 40 matching lines @@
       type: 'chrome',
       color: '#ffffff'
     },
     'innerBounds': {
       'width': INNER_WIDTH,
       'height': INNER_HEIGHT
     }
   };
   chrome.app.window.create('main.html', options, onWindowCreated);
 });