Fix threading bugs in product label UI.

chrome/browser/ui/webui/help/help_handler.cc

Index: chrome/browser/ui/webui/help/help_handler.cc
diff --git a/chrome/browser/ui/webui/help/help_handler.cc b/chrome/browser/ui/webui/help/help_handler.cc
index b88247f3f3061447b0ef0f9e38ef3f2e5bed1c6b..3417b036c7a93722d8e4ae2bc82b10e7e89b5f42 100644
--- a/chrome/browser/ui/webui/help/help_handler.cc
+++ b/chrome/browser/ui/webui/help/help_handler.cc
@@ -47,8 +47,10 @@
 #if defined(OS_CHROMEOS)
 #include "base/files/file_util_proxy.h"
 #include "base/i18n/time_formatting.h"
+#include "base/path_service.h"
 #include "base/prefs/pref_service.h"
 #include "base/sys_info.h"
+#include "base/task_runner_util.h"
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos.h"
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos_factory.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
@@ -58,6 +60,7 @@
 #include "chrome/browser/ui/webui/chromeos/image_source.h"
 #include "chrome/browser/ui/webui/help/help_utils_chromeos.h"
 #include "chrome/browser/ui/webui/help/version_updater_chromeos.h"
+#include "chromeos/chromeos_paths.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/power_manager_client.h"
@@ -126,6 +129,18 @@ bool CanChangeChannel(Profile* profile) {
   return false;
 }
 
+// Reads the file containing the FCC label text, if found.

[stevenjb, 2014/12/05 23:28:07]

nit: Comment that this is called from the blocking pool.

[michaelpg, 2014/12/08 00:58:19]

Done.

+std::string ReadFCCLabelText() {
+  base::FilePath shared_assets_dir;
+  std::string contents;
+  if (PathService::Get(chromeos::DIR_SHARED_ASSETS, &shared_assets_dir) &&

[satorux1, 2014/12/08 00:43:57]

Why not just use chrome::kChromeOSAssetPath?

Something like below should work.

const base::FilePath asset_dir(FILE_PATH_LITERAL(chrome::kChromeOSAssetPath));
const base::FilePath label_file_path = asset_dir.AppendASCII(kFCCLabelTextPath);

[michaelpg, 2014/12/08 00:58:19]

I don't know whether it would be better to use chrome/common/url_constants.h or
chromeos/chromeos_paths.cc. It looked like chromeos_paths is more idiomatic
because it relates to path on disk, not URL to get to that path.

But I'll do whatever you think is best.

[satorux1, 2014/12/08 01:12:27]

I'd suggest to keep using chrome::kChromeOSAssetPath. There are already some
other file path constants in url_constants.cc so removing kChromeOSAssetPath
alone doesn't make the code much cleaner. Besides, I think it'd be nicer to keep
this patch small to make it easier to merge.

[michaelpg, 2014/12/08 21:14:57]

Done.

+      base::ReadFileToString(shared_assets_dir.AppendASCII(kFCCLabelTextPath),
+                             &contents)) {
+    return contents;
+  }
+  return std::string();
+}
+
 #endif  // defined(OS_CHROMEOS)
 
 }  // namespace
@@ -403,9 +418,12 @@ void HelpHandler::OnPageLoaded(const base::ListValue* args) {
   version_updater_->GetChannel(false,
       base::Bind(&HelpHandler::OnTargetChannel, weak_factory_.GetWeakPtr()));
 
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&HelpHandler::LoadFCCLabelText, weak_factory_.GetWeakPtr()));
+  base::PostTaskAndReplyWithResult(
+      content::BrowserThread::GetBlockingPool(),
+      FROM_HERE,
+      base::Bind(&ReadFCCLabelText),
+      base::Bind(&HelpHandler::OnFCCLabelTextRead,
+                 weak_factory_.GetWeakPtr()));
 #endif
 }
 
@@ -587,16 +605,13 @@ void HelpHandler::OnTargetChannel(const std::string& channel) {
       "help.HelpPage.updateTargetChannel", base::StringValue(channel));
 }
 
-void HelpHandler::LoadFCCLabelText() {
-  base::FilePath path(std::string(chrome::kChromeOSAssetPath) +
-                      kFCCLabelTextPath);
-  std::string contents;
-  if (base::ReadFileToString(path, &contents)) {
-    // Remove unnecessary whitespace.
-    base::StringValue label(base::CollapseWhitespaceASCII(contents, true));
-    web_ui()->CallJavascriptFunction("help.HelpPage.setProductLabelText",
-                                     label);
-  }
+void HelpHandler::OnFCCLabelTextRead(std::string text) {
+  // Remove unnecessary whitespace.
+  text = base::CollapseWhitespaceASCII(text, true);
+  if (text.empty())
+    return;
+  base::StringValue label(text);
+  web_ui()->CallJavascriptFunction("help.HelpPage.setProductLabelText", label);
 }
 
 #endif // defined(OS_CHROMEOS)