arc: ARC loading progress should not be shown when started from OOBE.

chrome/browser/chromeos/arc/arc_support_host.h

 // 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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_ARC_ARC_SUPPORT_HOST_H_
 #define CHROME_BROWSER_CHROMEOS_ARC_ARC_SUPPORT_HOST_H_
 
 #include <memory>
 #include <string>
 
@@ skipping 34 matching lines @@
 
   // Observer to notify UI event.
   class Observer {
    public:
     virtual ~Observer() = default;
 
     // Called when the ARC support window is closed.
     virtual void OnWindowClosed() {}
 
     // Called when the user press AGREE button on ToS page.
-    virtual void OnTermsAgreed(bool is_metrics_enabled,
-                               bool is_backup_and_restore_enabled,
-                               bool is_location_service_enabled) {}
+    virtual void OnTermsAccepted(bool is_metrics_enabled,

[hidehiko, 2016/12/20 15:20:45]

Thank you for the refactoring. Could you make another CL to do this, to make the
focus clearer?

[khmel, 2016/12/20 16:27:58]

If we are renaming comments in this CL why not to rename methods as well.
Otherwise we have to rename both in separate CL.

+                                 bool is_backup_and_restore_enabled,
+                                 bool is_location_service_enabled) {}
 
     // Called when LSO auth token fetch is successfully completed.
     virtual void OnAuthSucceeded(const std::string& auth_code) {}
 
     // Called when "RETRY" button on the error page is clicked.
     virtual void OnRetryClicked() {}
 
     // Called when send feedback button on error page is clicked.
     virtual void OnSendFeedbackClicked() {}
   };
@@ skipping 26 matching lines @@
 
   // Requests to close the extension window.
   void Close();
 
   // Requests to show the "Terms Of Service" page.
   void ShowTermsOfService();
 
   // Requests to show the LSO page.
   void ShowLso();
 
-  // Requests to show the "ARC is loading" page.
-  void ShowArcLoading();
+  // Requests to show the "ARC is loading" page. If |force_activate| is set to
+  // true and UI is not shown this call activates UI first. If |force_activate|
+  // is set to false then this call does not activate UI and stores ui_page_
+  // state only.
+  void ShowArcLoading(bool force_activate);

[hidehiko, 2016/12/20 15:20:45]

The flag looks unnecessary complexity to me. Client code can control whether it
really needs to show the loading page or not.
I guess, you introduced this not to touch the |message_host_| outside of this
class, but in this class. However, it is what we should avoid to check the
member to decide whether loading page should be shown or not.

Please see also my comments in .cc file, too.

[khmel, 2016/12/20 16:27:58]

Responded in cc file.

 
   // Requests to show the error page
   void ShowError(Error error, bool should_show_send_feedback);
 
   void SetMetricsPreferenceCheckbox(bool is_enabled, bool is_managed);
   void SetBackupAndRestorePreferenceCheckbox(bool is_enabled, bool is_managed);
   void SetLocationServicesPreferenceCheckbox(bool is_enabled, bool is_managed);
 
   // arc::ArcSupportMessageHost::Observer override:
   void OnMessage(const base::DictionaryValue& message) override;
@@ skipping 67 matching lines @@
   bool is_arc_managed_ = false;
 
   PreferenceCheckboxData metrics_checkbox_;
   PreferenceCheckboxData backup_and_restore_checkbox_;
   PreferenceCheckboxData location_services_checkbox_;
 
   DISALLOW_COPY_AND_ASSIGN(ArcSupportHost);
 };
 
 #endif  // CHROME_BROWSER_CHROMEOS_ARC_ARC_SUPPORT_HOST_H_