Refactoring the SysInfoProvider.

chrome/browser/chromeos/system_logs/system_logs_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/system_logs/system_logs_fetcher.h"
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "chrome/browser/chromeos/system_logs/commandline_log_source.h"
+#include "chrome/browser/chromeos/system_logs/debugd_log_source.h"
+#include "chrome/browser/chromeos/system_logs/memorydetails_log_source.h"
+#include "chrome/browser/chromeos/system_logs/lsbrelease_log_source.h"
+#include "content/public/browser/browser_thread.h"
+
+using content::BrowserThread;
+
+namespace chromeos {
+
+SystemLogsFetcher::SystemLogsFetcher()
+    : request_(SysLogsFetcherCallback()),
+      response_(new SystemLogsResponse),

[satorux1, 2012/08/16 10:50:57]

when will it be deleted? make it a scoped_ptr? If you want to transfer the
ownership to the client, then pass the scoped_ptr to the callback, by
response_.Pass()

[tudalex(Chromium), 2012/08/16 22:33:03]

Done.

+      num_pending_requests_(0),
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
+
+  // Debug Daemon data source.
+  data_sources_.push_back(new DebugDaemonLogSource());
+
+  // Chrome data sources.
+  data_sources_.push_back(new CommandLineLogSource());
+  data_sources_.push_back(new LsbReleaseLogSource());
+  data_sources_.push_back(new MemoryDetailsLogSource());
+  num_pending_requests_ = data_sources_.size();
+}
+
+void SystemLogsFetcher::Fetch(const SysLogsFetcherCallback& request) {

[satorux1, 2012/08/16 10:50:57]

request -> callback

[tudalex(Chromium), 2012/08/16 22:33:03]

Done.

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  request_ = request;
+  for (size_t i = 0; i < data_sources_.size(); ++i) {
+    data_sources_[i]->Fetch(base::Bind(&SystemLogsFetcher::AddResponse,
+                                       weak_ptr_factory_.GetWeakPtr()));
+  }
+}
+
+void SystemLogsFetcher::AddResponse(SystemLogsResponse* response) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  for (SystemLogsResponse::iterator it = response->begin();
+       it != response->end();
+       ++it) {

[satorux1, 2012/08/16 10:50:57]

you might want to use size_t i = 0 here and elsewhere. it's more succinct. IMHO,
using an iterator for iterating a vector does not make much sense. it's just
longer and harder to read, but some people disagree, so it's a matter of taste.

[tudalex(Chromium), 2012/08/16 22:33:03]

response is a map and I don't know any ot her nice way to iterate over it. Also
realized map had a insert method that I can use to do this automatically.

[satorux1, 2012/08/17 17:46:29]

sorry, i wasn't aware this was a map. insert() lgtm.

+    (*response_)[it->first] = it->second;
+  }
+
+  --num_pending_requests_;
+  if (num_pending_requests_ > 0)
+    return;
+  request_.Run(response_);
+  BrowserThread::DeleteSoon(BrowserThread::UI, FROM_HERE, this);
+}
+
+}  // namespace chromeos
+