Integrate about:conflicts with --diagnostics

This should enable the user to find conflicting dlls that are crashing Chrome on startup.

This changelist introduces a limited_mode for scanning, since the diagnostics tests run without the help of many Chrome services, so we can't use the File thread for asynchronous scanning or the notification system.

Also changed the scanning so that if no signature/description is given in the blacklist, then we mark the DLL as a confirmed match if the name and the location match (before we used to match it as a 'suspected' match). A very sizable chunk of the blacklist is malware, which has no signature, so this gives us the ability to trigger the wrench badge on finding malware (that part remains opt-in through about:flags).

Also added failure count to diagnostics output (sometimes the error scrolls off screen, so having a note at the bottom helps).

BUG=51105
TEST=All diagnostics tests run as part of the unit tests.

Review URL: http://codereview.chromium.org/6098004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@70600 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 96 insertions, 21 deletions

diff --git a/chrome/browser/diagnostics/diagnostics_main.cc b/chrome/browser/diagnostics/diagnostics_main.cc
index 2f654c2..059dd59 100644
--- a/chrome/browser/diagnostics/diagnostics_main.cc
+++ b/chrome/browser/diagnostics/diagnostics_main.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -204,9 +204,14 @@ class TestWriter {
  public:
   // The |console| must be valid and properly initialized. This
   // class does not own it.
-  explicit TestWriter(SimpleConsole* console) : console_(console) {
+  explicit TestWriter(SimpleConsole* console)
+      : console_(console),
+        failures_(0) {
   }
 
+  // How many tests reported failure.
+  int failures() { return failures_; }
+
   // Write an informational line of text in white over black.
   bool WriteInfoText(const std::wstring& txt) {
     console_->SetColor(SimpleConsole::DEFAULT);
@@ -224,6 +229,7 @@ class TestWriter {
     } else {
       console_->SetColor(SimpleConsole::RED);
       console_->Write(L"[FAIL] ");
+      failures_++;
     }
     WriteInfoText(name + L"\n");
     std::wstring second_line(L"   ");
@@ -235,6 +241,9 @@ class TestWriter {
 
   SimpleConsole* console_;
 
+  // Keeps track of how many tests reported failure.
+  int failures_;
+
   DISALLOW_COPY_AND_ASSIGN(TestWriter);
 };
 
@@ -290,7 +299,12 @@ class TestController : public DiagnosticsModel::Observer {
   }
 
   virtual void OnDoneAll(DiagnosticsModel* model) {
-    writer_->WriteInfoText(L"DONE\n\n");
+    if (writer_->failures() > 0) {
+      writer_->WriteInfoText(StringPrintf(L"DONE. %d failure(s)\n\n",
+                             writer_->failures()));
+    } else {
+      writer_->WriteInfoText(L"DONE\n\n");
+    }
   }
 
  private:
diff --git a/chrome/browser/diagnostics/diagnostics_model.cc b/chrome/browser/diagnostics/diagnostics_model.cc
index b71f337d..209e342 100644
--- a/chrome/browser/diagnostics/diagnostics_model.cc
+++ b/chrome/browser/diagnostics/diagnostics_model.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -83,6 +83,7 @@ class DiagnosticsModelWin : public DiagnosticsModelImpl {
  public:
   DiagnosticsModelWin() {
     tests_.push_back(MakeOperatingSystemTest());
+    tests_.push_back(MakeConflictingDllsTest());
     tests_.push_back(MakeInstallTypeTest());
     tests_.push_back(MakeVersionTest());
     tests_.push_back(MakeUserDirTest());
diff --git a/chrome/browser/diagnostics/diagnostics_model.h b/chrome/browser/diagnostics/diagnostics_model.h
index 45c46e9..9b1e08a 100644
--- a/chrome/browser/diagnostics/diagnostics_model.h
+++ b/chrome/browser/diagnostics/diagnostics_model.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -12,7 +12,7 @@ class CommandLine;
 
 // The chrome diagnostics system is a model-view-controller system. The Model
 // responsible for holding and running the individual tests and providing a
-// uniform interface for quering the outcome.
+// uniform interface for querying the outcome.
 // TODO(cpu): The view and the controller are not yet built.
 class DiagnosticsModel {
  public:
@@ -33,7 +33,7 @@ class DiagnosticsModel {
    public:
     virtual ~Observer() {}
     // Called once upon test start with |percent| = 0 and periodically as the
-    // test progresses. There is no cancelation method.
+    // test progresses. There is no cancellation method.
     virtual void OnProgress(int id, int percent, DiagnosticsModel* model) = 0;
     // Called if the test in question cannot be run.
     virtual void OnSkipped(int id, DiagnosticsModel* model) = 0;
@@ -62,7 +62,7 @@ class DiagnosticsModel {
   virtual int GetTestRunCount() = 0;
   // Returns how many tests are available. This value never changes.
   virtual int GetTestAvailableCount() =0;
-  // Runs all the availabe tests, the |observer| callbacks will be called as
+  // Runs all the available tests, the |observer| callbacks will be called as
   // the test progress and thus cannot be null.
   virtual void RunAll(DiagnosticsModel::Observer* observer) = 0;
   // Get the information for a particular test. Do not keep a pointer to the
diff --git a/chrome/browser/diagnostics/diagnostics_model_unittest.cc b/chrome/browser/diagnostics/diagnostics_model_unittest.cc
index ee80a09..823ef9d 100644
--- a/chrome/browser/diagnostics/diagnostics_model_unittest.cc
+++ b/chrome/browser/diagnostics/diagnostics_model_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -78,7 +78,7 @@ class UTObserver: public DiagnosticsModel::Observer {
 
 // We currently have more tests operational on windows.
 #if defined(OS_WIN)
-const int kDiagnosticsTestCount = 18;
+const int kDiagnosticsTestCount = 19;
 #elif defined(OS_MACOSX)
 const int kDiagnosticsTestCount = 16;
 #elif defined(OS_POSIX)
diff --git a/chrome/browser/diagnostics/diagnostics_test.h b/chrome/browser/diagnostics/diagnostics_test.h
index 70d7eb7..298d5a2 100644
--- a/chrome/browser/diagnostics/diagnostics_test.h
+++ b/chrome/browser/diagnostics/diagnostics_test.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -16,7 +16,7 @@ class FilePath;
 // It also Implements the TestInfo interface providing the storage
 // for the outcome of the test.
 // Specific tests need (minimally) only to:
-// 1- override ExecuteImpl() to imnplement the test.
+// 1- override ExecuteImpl() to implement the test.
 // 2- call RecordStopFailure() or RecordFailure() or RecordSuccess()
 //    at the end of the test.
 // 3- Optionally call observer->OnProgress() if the test is long.
@@ -58,7 +58,7 @@ class DiagnosticTest : public DiagnosticsModel::TestInfo {
   static FilePath GetUserDefaultProfileDir();
 
  protected:
-  // The id needs to be overriden by derived classes and must uniquely
+  // The id needs to be overridden by derived classes and must uniquely
   // identify this test so other test can refer to it.
   virtual int GetId() = 0;
   // Derived classes override this method do perform the actual test.
diff --git a/chrome/browser/diagnostics/recon_diagnostics.cc b/chrome/browser/diagnostics/recon_diagnostics.cc
index f657fef..2cacd86 100644
--- a/chrome/browser/diagnostics/recon_diagnostics.cc
+++ b/chrome/browser/diagnostics/recon_diagnostics.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -22,6 +22,7 @@
 
 #if defined(OS_WIN)
 #include "base/win/windows_version.h"
+#include "chrome/browser/enumerate_modules_model_win.h"
 #include "chrome/installer/util/install_util.h"
 #endif
 
@@ -29,7 +30,7 @@
 // diagnostic tests. Here we check for the existence of critical files.
 // TODO(cpu): Define if it makes sense to localize strings.
 
-// TODO(cpu): There are a few maxium file sizes hardcoded in this file
+// TODO(cpu): There are a few maximum file sizes hardcoded in this file
 // that have little or no theoretical or experimental ground. Find a way
 // to justify them.
 
@@ -61,7 +62,7 @@ class OperatingSystemTest : public DiagnosticTest {
       return false;
     }
 #else
-    // TODO(port): define the OS criteria for linux and mac.
+    // TODO(port): define the OS criteria for Linux and Mac.
 #endif  // defined(OS_WIN)
     RecordSuccess(ASCIIToUTF16(StringPrintf("%s %s (%d [%d:%d])",
         base::SysInfo::OperatingSystemName().c_str(),
@@ -74,6 +75,60 @@ class OperatingSystemTest : public DiagnosticTest {
   DISALLOW_COPY_AND_ASSIGN(OperatingSystemTest);
 };
 
+// Check if any conflicting DLLs are loaded.
+class ConflictingDllsTest : public DiagnosticTest {
+ public:
+  ConflictingDllsTest() : DiagnosticTest(ASCIIToUTF16("Conflicting modules")) {}
+
+  virtual int GetId() { return 0; }
+
+  virtual bool ExecuteImpl(DiagnosticsModel::Observer* observer) {
+#if defined(OS_WIN)
+    EnumerateModulesModel* model = EnumerateModulesModel::GetInstance();
+    model->set_limited_mode(true);
+    model->ScanNow();
+    ListValue* list = model->GetModuleList();
+    if (!model->confirmed_bad_modules_detected() &&
+        !model->suspected_bad_modules_detected()) {
+      RecordSuccess(ASCIIToUTF16("No conflicting modules found"));
+      return true;
+    }
+
+    string16 failures = ASCIIToUTF16("Possibly conflicting modules:");
+    DictionaryValue* dictionary;
+    for (size_t i = 0; i < list->GetSize(); ++i) {
+      list->GetDictionary(i, &dictionary);
+      int status;
+      string16 location;
+      string16 name;
+      if (!dictionary->GetInteger("status", &status))
+        RecordFailure(ASCIIToUTF16("No 'status' field found"));
+      if (status < ModuleEnumerator::SUSPECTED_BAD)
+        continue;
+
+      if (!dictionary->GetString("location", &location)) {
+        RecordFailure(ASCIIToUTF16("No 'location' field found"));
+        return true;
+      }
+      if (!dictionary->GetString("name", &name)) {
+        RecordFailure(ASCIIToUTF16("No 'name' field found"));
+        return true;
+      }
+
+      failures += ASCIIToUTF16("\n") + location + name;
+    }
+    RecordFailure(failures);
+    return true;
+#else
+    RecordFailure(ASCIIToUTF16("Not implemented"));
+    return true;
+#endif  // defined(OS_WIN)
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ConflictingDllsTest);
+};
+
 // Check if it is system install or per-user install.
 class InstallTypeTest : public DiagnosticTest {
  public:
@@ -163,9 +218,9 @@ const TestPathInfo kPathsToTest[] = {
       true, false, false, 0}
 };
 
-// Check that the user's data directory exists and the paths are writeable.
-// If it is a systemwide install some paths are not expected to be writeable.
-// This test depends on |InstallTypeTest| having run succesfuly.
+// Check that the user's data directory exists and the paths are writable.
+// If it is a systemwide install some paths are not expected to be writable.
+// This test depends on |InstallTypeTest| having run successfully.
 class PathTest : public DiagnosticTest {
  public:
   explicit PathTest(const TestPathInfo& path_info)
@@ -227,7 +282,7 @@ class PathTest : public DiagnosticTest {
 };
 
 // Check that the disk space in the volume where the user data dir normally
-// lives is not dangerosly low.
+// lives is not dangerously low.
 class DiskSpaceTest : public DiagnosticTest {
  public:
   DiskSpaceTest() : DiagnosticTest(ASCIIToUTF16("Disk Space")) {}
@@ -337,6 +392,10 @@ DiagnosticTest* MakeOperatingSystemTest() {
   return new OperatingSystemTest();
 }
 
+DiagnosticTest* MakeConflictingDllsTest() {
+  return new ConflictingDllsTest();
+}
+
 DiagnosticTest* MakeInstallTypeTest() {
   return new InstallTypeTest();
 }
diff --git a/chrome/browser/diagnostics/recon_diagnostics.h b/chrome/browser/diagnostics/recon_diagnostics.h
index d7c52e9..8a20d4d 100644
--- a/chrome/browser/diagnostics/recon_diagnostics.h
+++ b/chrome/browser/diagnostics/recon_diagnostics.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -9,6 +9,7 @@
 #include "chrome/browser/diagnostics/diagnostics_test.h"
 
 DiagnosticTest* MakeOperatingSystemTest();
+DiagnosticTest* MakeConflictingDllsTest();
 DiagnosticTest* MakeInstallTypeTest();
 DiagnosticTest* MakeVersionTest();
 DiagnosticTest* MakeUserDirTest();