Default to current UI locale when recommended locales are invalid

chrome/browser/chromeos/login/screens/chrome_user_selection_screen.cc

 // Copyright 2014 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.
 
 #include "chrome/browser/chromeos/login/screens/chrome_user_selection_screen.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 119 matching lines @@
     // There are no recommended locales.
     PublicSessionRecommendedLocaleMap::iterator it =
         public_session_recommended_locales_.find(user_id);
     if (it != public_session_recommended_locales_.end()) {
       // If there previously were recommended locales, remove them from
       // |public_session_recommended_locales_| and notify the UI.
       public_session_recommended_locales_.erase(it);
       SetPublicSessionLocales(user_id, &new_recommended_locales);
     }
     return;
   }

[dzhioev (left Google), 2014/08/20 17:00:24]

Nothing will change if you drop this if-block completely.

[bartfab (slow), 2014/08/20 18:40:05]

The update sent to the UI is the same as in the next block. But the update of
|public_session_recommended_locales_| is different: The first block removes an
entry, the second adds/updates an entry.

[dzhioev (left Google), 2014/08/20 19:26:35]

Yes, but nothing will change, if you allow empty entries in
|public_session_recommended_locales_| instead of removing them.
I claim that removing first if-block will not change behavior of code.

[bartfab (slow), 2014/08/21 12:14:41]

I simplified this block a lot now. It is a tad less efficient but that makes no
difference in practice.

 
   // There are recommended locales.
   std::vector<std::string>& recommended_locales =
       public_session_recommended_locales_[user_id];
   if (new_recommended_locales != recommended_locales) {
     // If the list of recommended locales has changed, update
     // |public_session_recommended_locales_| and notify the UI.
     recommended_locales = new_recommended_locales;
     SetPublicSessionLocales(user_id, &new_recommended_locales);
   }
 }
 
 void ChromeUserSelectionScreen::SetPublicSessionDisplayName(
     const std::string& user_id) {
   const user_manager::User* user =
       user_manager::UserManager::Get()->FindUser(user_id);
   if (!user || user->GetType() != user_manager::USER_TYPE_PUBLIC_ACCOUNT)
     return;
 
   handler_->SetPublicSessionDisplayName(
       user_id,
       base::UTF16ToUTF8(user->GetDisplayName()));
 }
 
 void ChromeUserSelectionScreen::SetPublicSessionLocales(
     const std::string& user_id,
     const std::vector<std::string>* recommended_locales) {

[dzhioev (left Google), 2014/08/20 17:00:24]

This method is never called with |recommended_locales == NULL| so you can use
const ref instead of const ptr.

[bartfab (slow), 2014/08/20 18:40:05]

Done.

   if (!handler_initialized_)
     return;
 
   // Construct the list of available locales. This list consists of the
   // recommended locales, followed by all others.
-  scoped_ptr<base::ListValue> locales =
+  scoped_ptr<base::ListValue> available_locales =
       GetUILanguageList(recommended_locales, std::string());
 
-  // Set the initially selected locale. If the list of recommended locales is
-  // not empty, select its first entry. Otherwise, select the current UI locale.
-  const std::string& default_locale = recommended_locales->empty() ?
-      g_browser_process->GetApplicationLocale() : recommended_locales->front();
+  // Set the initially selected locale to the first recommended locale that is
+  // actually available or the current UI locale if none of them are available.
+  const std::string default_locale = FindMostRelevantLocale(
+      recommended_locales,
+      *available_locales.get(),
+      g_browser_process->GetApplicationLocale());
 
   // Set a flag to indicate whether the list of recommended locales contains at
   // least two entries. This is used to decide whether the public session pod
   // expands to its basic form (for zero or one recommended locales) or the
   // advanced form (two or more recommended locales).
   const bool two_or_more_recommended_locales = recommended_locales->size() >= 2;
 
   // Notify the UI.
   handler_->SetPublicSessionLocales(user_id,
-                                    locales.Pass(),
+                                    available_locales.Pass(),
                                     default_locale,
                                     two_or_more_recommended_locales);
 }
 
 }  // namespace chromeos