Unconditionally send Chrome and Chrome Frame events through ETW machinery.

Previously, these components only registered themselves if CHROME_ETW_LOGGING was set in the environment.  This is likely preventing the new log machinery in http://crrev.com/126240 from providing useful information on the main waterfall bots.

Siggi: overall
Brett: chrome/common

BUG=none
TEST=none
R=siggi@chromium.org,brettw@chromium.org

Review URL: https://chromiumcodereview.appspot.com/9702021

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

chrome/common/env_vars.cc

@@ -17,10 +17,6 @@ const char kLogFileName[] = "CHROME_LOG_FILE";
 // The name of the session log directory when logged in to ChromeOS.
 const char kSessionLogDir[] = "CHROMEOS_SESSION_LOG_DIR";
 
-// If this environment variable is set, Chrome on Windows will log
-// to Event Tracing for Windows.
-const char kEtwLogging[] = "CHROME_ETW_LOGGING";
-
 // CHROME_CRASHED exists if a previous instance of chrome has crashed. This
 // triggers the 'restart chrome' dialog. CHROME_RESTART contains the strings
 // that are needed to show the dialog.

chrome/common/env_vars.h

@@ -13,7 +13,6 @@ namespace env_vars {
 extern const char kHeadless[];
 extern const char kLogFileName[];
 extern const char kSessionLogDir[];
-extern const char kEtwLogging[];
 extern const char kShowRestart[];
 extern const char kRestartInfo[];
 extern const char kRtlLocale[];

chrome/common/logging_chrome.cc

@@ -369,8 +369,7 @@ void InitChromeLogging(const CommandLine& command_line,
 
 #if defined(OS_WIN)
   // Enable trace control and transport through event tracing for Windows.
-  if (env->HasVar(env_vars::kEtwLogging))
-    logging::LogEventProvider::Initialize(kChromeTraceProviderName);
+  logging::LogEventProvider::Initialize(kChromeTraceProviderName);
 #endif
 
 #ifdef NDEBUG

chrome/test/logging/win/file_logger.cc

@@ -18,7 +18,6 @@
 #include "base/utf_string_conversions.h"
 #include "base/win/event_trace_consumer.h"
 #include "base/win/registry.h"
-#include "chrome/common/env_vars.h"
 
 namespace logging_win {
 
@@ -58,71 +57,8 @@ COMPILE_ASSERT((1 << arraysize(kProviders)) - 1 ==
                    FileLogger::kAllEventProviders,
                size_of_kProviders_is_inconsistent_with_kAllEventProviders);
 
-// The provider bits that require CHROME_ETW_LOGGING in the environment.
-const uint32 kChromeLogProviders =
-    FileLogger::CHROME_LOG_PROVIDER | FileLogger::CHROME_FRAME_LOG_PROVIDER;
-const HKEY kEnvironmentRoot = HKEY_LOCAL_MACHINE;
-const wchar_t kEnvironmentKey[] =
-    L"SYSTEM\\CurrentControlSet\\Control\\Session Manager\\Environment";
-const wchar_t kEnvironment[] = L"Environment";
-const unsigned int kBroadcastTimeoutMilliseconds = 2 * 1000;
-
 }  // namespace
 
-// FileLogger::ScopedSystemEnvironmentVariable implementation.
-
-FileLogger::ScopedSystemEnvironmentVariable::ScopedSystemEnvironmentVariable(
-    const string16& variable,
-    const string16& value) {
-
-  // Set the value in this process and its children.
-  ::SetEnvironmentVariable(variable.c_str(), value.c_str());
-
-  // Set the value for the whole system and ask everyone to refresh.
-  base::win::RegKey environment;
-  LONG result = environment.Open(kEnvironmentRoot, kEnvironmentKey,
-                                 KEY_QUERY_VALUE | KEY_SET_VALUE);
-  if (result == ERROR_SUCCESS) {
-    string16 old_value;
-    // The actual value of the variable is insignificant in the eyes of Chrome.
-    if (environment.ReadValue(variable.c_str(),
-                              &old_value) != ERROR_SUCCESS &&
-        environment.WriteValue(variable.c_str(),
-                               value.c_str()) == ERROR_SUCCESS) {
-      environment.Close();
-      // Remember that this needs to be reversed in the dtor.
-      variable_ = variable;
-      NotifyOtherProcesses();
-    }
-  } else {
-    SetLastError(result);
-    PLOG(ERROR) << "Failed to open HKLM to check/modify the system environment";
-  }
-}
-
-FileLogger::ScopedSystemEnvironmentVariable::~ScopedSystemEnvironmentVariable()
-{
-  if (!variable_.empty()) {
-    base::win::RegKey environment;
-    if (environment.Open(kEnvironmentRoot, kEnvironmentKey,
-                         KEY_SET_VALUE) == ERROR_SUCCESS) {
-      environment.DeleteValue(variable_.c_str());
-      environment.Close();
-      NotifyOtherProcesses();
-    }
-  }
-}
-
-// static
-void FileLogger::ScopedSystemEnvironmentVariable::NotifyOtherProcesses() {
-  // Announce to the system that a change has been made so that the shell and
-  // other Windowsy bits pick it up; see
-  // http://msdn.microsoft.com/en-us/library/windows/desktop/ms682653.aspx.
-  SendMessageTimeout(HWND_BROADCAST, WM_SETTINGCHANGE, 0,
-                     reinterpret_cast<LPARAM>(kEnvironment), SMTO_ABORTIFHUNG,
-                     kBroadcastTimeoutMilliseconds, NULL);
-}
-
 bool FileLogger::is_initialized_ = false;
 
 FileLogger::FileLogger()
@@ -197,13 +133,6 @@ void FileLogger::Initialize() {
 void FileLogger::Initialize(uint32 event_provider_mask) {
   CHECK(!is_initialized_);
 
-  // Set up CHROME_ETW_LOGGING in the environment if providers that require it
-  // are enabled.
-  if (event_provider_mask & kChromeLogProviders) {
-    etw_logging_configurator_.reset(new ScopedSystemEnvironmentVariable(
-        ASCIIToWide(env_vars::kEtwLogging), L"1"));
-  }
-
   // Stop a previous session that wasn't shut down properly.
   base::win::EtwTraceProperties ignore;
   HRESULT hr = base::win::EtwTraceController::Stop(kChromeTestSession,

chrome/test/logging/win/file_logger.h

@@ -29,12 +29,6 @@ namespace logging_win {
 // - This class is not thread safe.
 // - This class uses facilities that require the process to run with admin
 //   rights; StartLogging() will return false if this is not the case.
-//
-// Initializing an instance will add the variable CHROME_ETW_LOGGING=1 to the
-// system-wide environment if it is not present.  In the case where it is not
-// already present, log messages will not be captured from currently running
-// instances of Chrome, Chrome Frame, or other providers that generate events
-// conditionally based on that environment variable.
 class FileLogger {
  public:
   enum EventProviderBits {
@@ -78,30 +72,11 @@ class FileLogger {
   }
 
  private:
-  // A helper for setting/clearing a variable in the system-wide environment.
-  class ScopedSystemEnvironmentVariable {
-   public:
-    ScopedSystemEnvironmentVariable(const string16& variable,
-                                    const string16& value);
-    ~ScopedSystemEnvironmentVariable();
-
-   private:
-    static void NotifyOtherProcesses();
-
-    // Non-empty if the variable was inserted into the system-wide environment.
-    string16 variable_;
-    DISALLOW_COPY_AND_ASSIGN(ScopedSystemEnvironmentVariable);
-  };
-
   bool EnableProviders();
   void DisableProviders();
 
-  void ConfigureChromeEtwLogging();
-  void RevertChromeEtwLogging();
-
   static bool is_initialized_;
 
-  scoped_ptr<ScopedSystemEnvironmentVariable> etw_logging_configurator_;
   base::win::EtwTraceController controller_;
   uint32 event_provider_mask_;
 

chrome_frame/chrome_tab.cc

@@ -28,7 +28,6 @@
 #include "base/win/windows_version.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome/common/env_vars.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "chrome_frame/bho.h"
 #include "chrome_frame/chrome_active_document.h"
@@ -856,10 +855,7 @@ extern "C" BOOL WINAPI DllMain(HINSTANCE instance,
     }
 
     // Enable trace control and transport through event tracing for Windows.
-    if (::GetEnvironmentVariable(ASCIIToWide(env_vars::kEtwLogging).c_str(),
-                                 NULL, 0)) {
-      logging::LogEventProvider::Initialize(kChromeFrameProvider);
-    }
+    logging::LogEventProvider::Initialize(kChromeFrameProvider);
 
     // Initialize the field test infrastructure. Must be done somewhere that
     // can only get called once. For Chrome Frame, that is here.