Refactoring the SysInfoProvider.

chrome/browser/chromeos/syslogs/commandline_fetcher.cc

+// Copyright (c) 2012 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/syslogs/commandline_fetcher.h"
+
+#include <string>
+#include <vector>
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/command_line.h"
+#include "base/file_path.h"
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "base/process_util.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace chromeos {
+
+void CommandLineFetcher::Fetch(const SysLogsFetcherCallback& request) {
+  if (!content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE)) {
+    content::BrowserThread::PostTask(content::BrowserThread::FILE,

[satorux1, 2012/08/08 22:01:03]

FILE thread is supposedly dead. Please use PostBlockingPoolTask or friends
instead

[tudalex(Chromium), 2012/08/09 02:30:24]

Done.

+                                     FROM_HERE,
+                                     base::Bind(&CommandLineFetcher::Fetch,
+                                                base::Unretained(this),

[satorux1, 2012/08/08 22:01:03]

This looks unsafe. Is this guaranteed that |this| is alive when the callback is
run on the FILE thread? Otherwise, |this| can be referenced after it's deleted,
which is a use-after-free.

AFAIK, the best way to solve this is to use PostTaskAndReply idiom.


 Result* result = new Result;
 PostBlockingPoolTaskAndReply(
     FROM_HERE,
     base::Bind(&FreeFunction, result),
     base::Bind(&Foo::MemberFunction,
                weak_ptr_factory_.GetWeakPtr(),
                base::Owned(result)));

[tudalex(Chromium), 2012/08/09 02:30:24]

Created a anonymous namespace and put the main execution part on a function
there.

+                                                request));
+    return;
+  }
+
+  // TODO(tudalex): Move program calling in a array or something similar to make
+  // it more easier to modify and understand.
+  std::vector<std::pair<std::string, CommandLine> > commands;
+
+  // Needed to initialize an empty command, but the constructor was private
+  CommandLine command(FilePath("/usr/bin/cras_test_client"));
+  command.AppendArg("--dump_server_info");
+  commands.push_back(std::make_pair("cras", command));
+
+  command = CommandLine((FilePath("set")));
+  commands.push_back(std::make_pair("env", command));
+
+  command = CommandLine(FilePath("/usr/bin/setxkbmap"));
+  command.AppendArg("-print");
+  command.AppendArg("-query");
+  commands.push_back(std::make_pair("setxkbmap", command));
+
+  command = CommandLine(FilePath("/usr/bin/xrandr"));
+  command.AppendArg("--verbose");
+  commands.push_back(std::make_pair("xrandr", command));
+
+  command = CommandLine(FilePath("/opt/google/touchpad/tpcontrol"));
+  commands.push_back(std::make_pair("hack-33025-touchpad", command));
+
+  command = CommandLine(FilePath("/opt/google/touchpad/generate_userfeedback"));
+  commands.push_back(std::make_pair("hack-33025-touchpad_activity", command));
+
+
+  std::vector< std::pair<std::string, CommandLine> >::iterator it;
+  for (it = commands.begin(); it != commands.end(); ++it) {
+    std::string output;
+    base::GetAppOutput(it->second, &output);
+    (*response_)[it->first] = output;
+  }
+  content::BrowserThread::PostTask(content::BrowserThread::UI,
+                                     FROM_HERE,

[satorux1, 2012/08/08 22:01:03]

indentation.

[tudalex(Chromium), 2012/08/09 02:30:24]

Done.

+                                     base::Bind(request, response_));
+}
+
+}  // namespace chromeos