[Win] Add reporting of total number of modules loaded in browser process.

chrome/browser/enumerate_modules_model_win.h

 // 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.
 
 #ifndef CHROME_BROWSER_ENUMERATE_MODULES_MODEL_WIN_H_
 #define CHROME_BROWSER_ENUMERATE_MODULES_MODEL_WIN_H_
 
 #include <utility>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/singleton.h"
+#include "base/observer_list_threadsafe.h"
 #include "base/strings/string16.h"
 #include "base/timer/timer.h"
 #include "content/public/browser/browser_thread.h"
 #include "url/gurl.h"
 
 class EnumerateModulesModel;
 
 namespace base {
 class FilePath;
 class ListValue;
@@ skipping 83 matching lines @@
     // The duplicate count within each category of modules.
     int duplicate_count;
     // Whether this module has been normalized (necessary before checking it
     // against blacklist).
     bool normalized;
   };
 
   // A vector typedef of all modules enumerated.
   typedef std::vector<Module> ModulesVector;
 
-  // A structure we populate with the blacklist entries.
-  struct BlacklistEntry {
-    const char* filename;
-    const char* location;
-    const char* desc_or_signer;
-    const char* version_from;  // Version where conflict started.
-    const char* version_to;    // First version that works.
-    OperatingSystem os;  // Bitmask, representing what OS this entry applies to.
-    RecommendedAction help_tip;
-  };
-
   // A static function that normalizes the module information in the |module|
   // struct. Module information needs to be normalized before comparing against
   // the blacklist. This is because the same module can be described in many
   // different ways, ie. file paths can be presented in long/short name form,
   // and are not case sensitive on Windows. Also, the version string returned
   // can include appended text, which we don't want to use during comparison
   // against the blacklist.
   static void NormalizeModule(Module* module);
 
-  // A static function that checks whether |module| has been |blacklisted|.
-  static ModuleStatus Match(const Module& module,
-                            const BlacklistEntry& blacklisted);
-
   explicit ModuleEnumerator(EnumerateModulesModel* observer);
 
   // Start scanning the loaded module list (if a scan is not already in
   // progress). This function does not block while reading the module list
   // (unless we are in limited_mode, see below), and will notify when done
   // through the MODULE_LIST_ENUMERATED notification.

[grt (UTC plus 2), 2016/06/16 18:27:53]

update doc comments regarding notifications throughout

[chrisha, 2016/06/17 20:48:03]

Done.

   // The process will also send MODULE_INCOMPATIBILITY_BADGE_CHANGE to let
   // observers know when it is time to update the wrench menu badge.
   // When in |limited_mode|, this function will not leverage the File thread
   // to run asynchronously and will therefore block until scanning is done
   // (and will also not send out any notifications).
   void ScanNow(ModulesVector* list, bool limited_mode);

[grt (UTC plus 2), 2016/06/16 18:27:53]

i can't find any callers that use limited_mode=true. remove support for this?

[chrisha, 2016/06/17 20:48:02]

chrome/browser/diagnostics/recon_diagnostics.cc still uses this.

I'm not sure how important that use case still is, given the general state of
bitrot in this code. I'll track down the appropriate folks, but I'd prefer to
not break them in the meantime.

 
  private:
   FRIEND_TEST_ALL_PREFIXES(EnumerateModulesTest, CollapsePath);
 
   friend class base::RefCountedThreadSafe<ModuleEnumerator>;
   ~ModuleEnumerator();
 
-  // The (currently) hard coded blacklist of known bad modules.
-  static const BlacklistEntry kModuleBlacklist[];
-
   // This function does the actual file scanning work on the FILE thread (or
   // block the main thread when in limited_mode). It enumerates all loaded
   // modules in the process and other modules of interest, such as the
   // registered Winsock LSP modules and stores them in |enumerated_modules_|.
   // It then normalizes the module info and matches them against a blacklist
   // of known bad modules. Finally, it calls ReportBack to let the observer
   // know we are done.
   void ScanImpl();
 
   // Enumerate all modules loaded into the Chrome process.
@@ skipping 24 matching lines @@
   // we can use for comparison against our blacklist (which uses only env vars).
   // NOTE: The vector will not contain an exhaustive list of environment
   // variables, only the ones currently found on the blacklist or ones that are
   // likely to appear there.
   void PreparePathMappings();
 
   // For a given |module|, collapse the path from c:\windows to %systemroot%,
   // based on the |path_mapping_| vector.
   void CollapsePath(Module* module);
 
-  // Takes each module in the |enumerated_modules_| vector and matches it
-  // against a fixed blacklist of bad and suspected bad modules.
-  void MatchAgainstBlacklist();
-
   // This function executes on the UI thread when the scanning and matching
   // process is done. It notifies the observer.
   void ReportBack();
 
   // Given a filename, returns the Subject (who signed it) retrieved from
   // the digital signature (Authenticode).
   base::string16 GetSubjectNameFromDigitalSignature(
       const base::FilePath& filename);
 
+  void ReportThirdPartyMetrics();

[grt (UTC plus 2), 2016/06/16 18:27:53]

please add doc comment

[chrisha, 2016/06/17 20:48:03]

Done.

+
   // The typedef for the vector that maps a regular file path to %env_var%.
-  typedef std::vector< std::pair<base::string16, base::string16> > PathMapping;
+  typedef std::vector<std::pair<base::string16, base::string16>> PathMapping;
 
   // The vector of paths to %env_var%, used to account for differences in
   // where people keep there files, c:\windows vs. d:\windows, etc.
   PathMapping path_mapping_;
 
   // The vector containing all the enumerated modules (loaded and modules of
   // interest).
   ModulesVector* enumerated_modules_;
 
-  // The observer, who needs to be notified when we are done.
+  // The observers, who need to be notified when the scan is complete.
   EnumerateModulesModel* observer_;

[grt (UTC plus 2), 2016/06/16 18:27:53]

why is the pointer to the model called "observer_"?

[chrisha, 2016/06/17 20:48:02]

The implementation of the enumeration is done by this helper class
(ModuleEnumerator), and it notifies the EnumerateModulesModel when it has
completed. The 'observer' is passed in via the constructor.

This could stand to be cleaned up, but I was hoping to avoid refactoring the
whole world.

 
   // See limited_mode below.
   bool limited_mode_;
 
   // The thread that we need to call back on to report that we are done.
   content::BrowserThread::ID callback_thread_id_;
 
   DISALLOW_COPY_AND_ASSIGN(ModuleEnumerator);
 };
 
@@ skipping 13 matching lines @@
 class EnumerateModulesModel {
  public:
   // UMA histogram constants.
   enum UmaModuleConflictHistogramOptions {
     ACTION_BUBBLE_SHOWN = 0,
     ACTION_BUBBLE_LEARN_MORE,
     ACTION_MENU_LEARN_MORE,
     ACTION_BOUNDARY, // Must be the last value.
   };
 
+  // Observer class used to receive the list of modules when enumeration is
+  // finished.
+  class Observer {

[grt (UTC plus 2), 2016/06/16 18:27:53]

the following suggestions are for consistency with observers in content/public
that do not carry behavior

[chrisha, 2016/06/17 20:48:03]

Done.

+   public:
+    Observer() {}

[grt (UTC plus 2), 2016/06/16 18:27:53]

omit the ctor

[chrisha, 2016/06/17 20:48:02]

Done.

+    virtual ~Observer() {}

[grt (UTC plus 2), 2016/06/16 18:27:53]

move dtor into "protected:" block i think " = default;" is a good choice here as
well

[chrisha, 2016/06/17 20:48:03]

Done.

+
+    // Invoked when EnumerateModulesModel has completed a scan of modules.
+    virtual void OnScanCompleted(bool limited_mode);

[grt (UTC plus 2), 2016/06/16 18:27:53]

inline the empty methods with "{}"

[chrisha, 2016/06/17 20:48:03]

Done.

+
+    // Invoked when a user has acknowledge incompatible modules found in a
+    // module scan.
+    virtual void OnConflictsAcknowledged();
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Observer);

[grt (UTC plus 2), 2016/06/16 18:27:53]

omit this

[chrisha, 2016/06/17 20:48:03]

Done.

+  };
+
   static EnumerateModulesModel* GetInstance();
 
+  // Adds an |observer| to the enumerator. May be called from any thread. The
+  // callback will be invoked on the thread on which AddObserver was called.
+  void AddObserver(Observer* observer) {
+    observers_->AddObserver(observer);
+  }
+
+  // Removes an |observer| from the enumerator. May be called from any thread.
+  void RemoveObserver(Observer* observer) {
+    observers_->RemoveObserver(observer);
+  }
+
   // Returns true if we should show the conflict notification. The conflict
   // notification is only shown once during the lifetime of the process.
   bool ShouldShowConflictWarning() const;
 
   // Called when the user has acknowledged the conflict notification.
   void AcknowledgeConflictNotification();
 
   // Returns the number of suspected bad modules found in the last scan.
   // Returns 0 if no scan has taken place yet.
   int suspected_bad_modules_detected() const {
@@ skipping 20 matching lines @@
   // Checks to see if a scanning task should be started and sets one off, if so.
   void MaybePostScanningTask();
 
   // Asynchronously start the scan for the loaded module list, except when in
   // limited_mode (in which case it blocks).
   void ScanNow();
 
   // Gets the whole module list as a ListValue.
   base::ListValue* GetModuleList() const;
 
-  // Gets the Help Center URL for the first *notable* conflict module that we've
-  // elected to notify the user about.
-  GURL GetFirstNotableConflict();
+  // The user will be taken to this site when the conflict bubble or app menu

[grt (UTC plus 2), 2016/06/16 18:27:53]

nit:
  // Returns the site to which the user should be taken when...
as per https://google.github.io/styleguide/cppguide.html#Function_Comments

[chrisha, 2016/06/17 20:48:03]

Done.

+  // item is clicked. For now this simply opens chrome://conflicts, which
+  // contains detailed information about conflicts. Returns an empty URL if
+  // there are no conficts.
+  GURL GetConflictUrl();
 
  private:
   friend struct base::DefaultSingletonTraits<EnumerateModulesModel>;
   friend class ModuleEnumerator;
 
   EnumerateModulesModel();
   virtual ~EnumerateModulesModel();
 
   // Called on the UI thread when the helper class is done scanning.
   void DoneScanning();
 
-  // Constructs a Help Center article URL for help with a particular module.
-  // The module must have the SEE_LINK attribute for |recommended_action| set,
-  // otherwise this returns a blank string.
-  GURL ConstructHelpCenterUrl(const ModuleEnumerator::Module& module) const;
-
   // The vector containing all the modules enumerated. Will be normalized and
   // any bad modules will be marked.
   ModuleEnumerator::ModulesVector enumerated_modules_;
 
   // The object responsible for enumerating the modules on the File thread.
   scoped_refptr<ModuleEnumerator> module_enumerator_;
 
   // When this singleton object is constructed we go and fire off this timer to

[grt (UTC plus 2), 2016/06/16 18:27:53]

this comment is a lie

[chrisha, 2016/06/17 20:48:03]

Done.

   // start scanning for modules after a certain amount of time has passed.
   base::OneShotTimer check_modules_timer_;
 
   // While normally |false|, this mode can be set to indicate that the scanning
   // process should not rely on certain services normally available to Chrome,
   // such as the resource bundle and the notification system, not to mention
   // having multiple threads. This mode is useful during diagnostics, which
   // runs without firing up all necessary Chrome services first.
   bool limited_mode_;
 
   // True if we are currently scanning for modules.
   bool scanning_;
 
   // Whether the conflict notification has been acknowledged by the user.
   bool conflict_notification_acknowledged_;
 
   // The number of confirmed bad modules (not including suspected bad ones)
   // found during last scan.
   int confirmed_bad_modules_detected_;
 
   // The number of bad modules the user needs to be aggressively notified about.
   int modules_to_notify_about_;
 
   // The number of suspected bad modules (not including confirmed bad ones)
   // found during last scan.
   int suspected_bad_modules_detected_;
 
+  scoped_refptr<base::ObserverListThreadSafe<Observer>> observers_;
+
   DISALLOW_COPY_AND_ASSIGN(EnumerateModulesModel);
 };
 
 #endif  // CHROME_BROWSER_ENUMERATE_MODULES_MODEL_WIN_H_