Refactoring the SysInfoProvider.

chrome/browser/chromeos/syslogs/syslogs_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/syslogs_fetcher.h"
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "chrome/browser/chromeos/syslogs/commandline_fetcher.h"
+#include "chrome/browser/chromeos/syslogs/debugd_log_fetcher.h"
+#include "chrome/browser/chromeos/syslogs/memorydetails_fetcher.h"
+#include "chrome/browser/chromeos/syslogs/lsbrelease_fetcher.h"
+#include "content/public/browser/browser_thread.h"
+
+
+using content::BrowserThread;
+
+namespace chromeos {
+
+
+SysLogsFetcher::SysLogsFetcher()
+    : response_(new SysLogsResponse),
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {

[satorux1, 2012/08/09 21:15:07]

Please be sure to initialize members of primitive types initializer list. Here,
responses_ is not initialized.

Please check other classes too.

[tudalex(Chromium), 2012/08/10 00:30:31]

Done.

+
+  // Debug Daemon data source.
+  data_sources_.push_back(new DebugDaemonLogFetcher());
+
+  // Chrome data sources.
+  data_sources_.push_back(new CommandLineFetcher());
+  data_sources_.push_back(new LSBReleaseFetcher());
+  data_sources_.push_back(new MemoryDetailsFetcher());
+  responses_ = data_sources_.size();

[satorux1, 2012/08/09 21:15:07]

maybe responses_ is unnecessary?  cannot we just use data_sources_.size()?

[tudalex(Chromium), 2012/08/10 00:30:31]

I'm decrementing responses_ for each response I've recieved I can't do that with
data_soruces_.size().

[satorux1, 2012/08/10 22:58:34]

Then, please name it to something like num_pending_requests_

[tudalex(Chromium), 2012/08/11 00:27:06]

Done.

+}
+
+void SysLogsFetcher::Fetch(const SysLogsFetcherCallback& request) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

[satorux1, 2012/08/09 21:15:07]

blank line here.

[tudalex(Chromium), 2012/08/10 00:30:31]

Done.

+  request_ = request;
+  SysLogsDataSources::iterator it;
+  responses_ = data_sources_.size();
+  for (it = data_sources_.begin(); it != data_sources_.end(); ++it) {

[satorux1, 2012/08/09 21:15:07]

for (SysLogsDataSources::iterator it = data_sources_.begin(); 

[tudalex(Chromium), 2012/08/10 00:30:31]

Done.

+    (*it)->Fetch(base::Bind(&SysLogsFetcher::AddData,
+                              weak_ptr_factory_.GetWeakPtr()));
+  }
+}
+
+void SysLogsFetcher::AddData(SysLogsResponse* data) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

[satorux1, 2012/08/09 21:15:07]

blank line here.

[tudalex(Chromium), 2012/08/10 00:30:31]

Done.

+  SysLogsResponse::iterator it;
+  for (it = data->begin(); it != data->end(); ++it){

[satorux1, 2012/08/09 21:15:07]

for (SysLogsResponse::iterator it; ...)

[tudalex(Chromium), 2012/08/10 00:30:31]

Done.

+    (*response_)[it->first] = it->second;
+  }
+  delete data;
+
+  if (--responses_)
+    return;
+
+  SysLogsDataSources::iterator ds_it;

[satorux1, 2012/08/09 21:15:07]

then, you don't need this weird name.

[tudalex(Chromium), 2012/08/10 00:30:31]

Done.

+  for (ds_it = data_sources_.begin(); ds_it != data_sources_.end(); ++ds_it) {
+    delete *ds_it;
+  }
+  request_.Run(response_);
+  BrowserThread::DeleteSoon(BrowserThread::UI, FROM_HERE, this);
+}
+
+}  // namespace chromeos
+