Merge 33269 - Revert 33255 Report active extensions in crash reports. This only implements Windows right now. Mac and linux will be separate CLs.

"Active" is overloaded to mean different things depending on
the process type:

 browser: all enabled extensions
 renderer: unique set of extensions from all user scripts
 extension: extensions running in the process

BUG=27169

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



There are thousands of new crashes with reivison 33256 and 33255, so i'm
reverting. the crashes are in chrome_2610000!child_process_logging::SetActiveURL



TBR=aa@chromium.org
Review URL: http://codereview.chromium.org/448006

TBR=nsylvain@chromium.org
Review URL: http://codereview.chromium.org/451005

git-svn-id: svn://svn.chromium.org/chrome/branches/249/src@33274 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 50 insertions, 180 deletions

diff --git a/chrome/app/breakpad_win.cc b/chrome/app/breakpad_win.cc
index 90cf549..297a034 100644
--- a/chrome/app/breakpad_win.cc
+++ b/chrome/app/breakpad_win.cc
@@ -33,10 +33,10 @@ const wchar_t kSystemPrincipalSid[] =L"S-1-5-18";
 
 google_breakpad::ExceptionHandler* g_breakpad = NULL;
 
-// Pointers to memory that will be sent in crash reports. These are kept updated
-// over the life of the process.
 std::vector<wchar_t*>* g_url_chunks = NULL;
-std::vector<wchar_t*>* g_extension_ids = NULL;
+
+// A string containing the user's unique metric services id. We send this
+// in the crash report.
 wchar_t* g_client_id = NULL;
 
 // Dumps the current process memory.
@@ -74,69 +74,69 @@ google_breakpad::CustomClientInfo* GetCustomInfo(const std::wstring& dll_path,
      version = L"0.0.0.0-devel";
   }
 
-  const int kNumCustomInfoEntries = 17;
-  static std::vector<google_breakpad::CustomInfoEntry> entries;
-
-  // We save pointers into this as we're building it up and we don't want them
-  // to become invalid due to resizes.
-  entries.reserve(kNumCustomInfoEntries);
-
   // Common entries.
-  entries.push_back(google_breakpad::CustomInfoEntry(L"ver", version.c_str()));
-  entries.push_back(google_breakpad::CustomInfoEntry(L"plat", L"Win32"));
-  entries.push_back(google_breakpad::CustomInfoEntry(L"ptype", type.c_str()));
-
-  g_extension_ids = new std::vector<wchar_t*>(kMaxReportedActiveExtensions);
-  for (int i = 0; i < kMaxReportedActiveExtensions; ++i) {
-    entries.push_back(google_breakpad::CustomInfoEntry(
-        StringPrintf(L"extension-%i", i + 1).c_str(), L""));
-    (*g_extension_ids)[i] = entries.back().value;
-  }
+  google_breakpad::CustomInfoEntry ver_entry(L"ver", version.c_str());
+  google_breakpad::CustomInfoEntry prod_entry(L"prod", product.c_str());
+  google_breakpad::CustomInfoEntry plat_entry(L"plat", L"Win32");
+  google_breakpad::CustomInfoEntry type_entry(L"ptype", type.c_str());
 
   // Read the id from registry. If reporting has never been enabled
   // the result will be empty string. Its OK since when user enables reporting
   // we will insert the new value at this location.
   std::wstring guid;
   GoogleUpdateSettings::GetMetricsId(&guid);
-  entries.push_back(google_breakpad::CustomInfoEntry(L"guid", guid.c_str()));
-  g_client_id = entries.back().value;
+  google_breakpad::CustomInfoEntry guid_entry(L"guid", guid.c_str());
 
   if (type == L"renderer" || type == L"plugin") {
     // Create entries for the URL. Currently we only allow each chunk to be 64
     // characters, which isn't enough for a URL. As a hack we create 8 entries
     // and split the URL across the entries.
-    g_url_chunks = new std::vector<wchar_t*>(kMaxUrlChunks);
-    for (int i = 0; i < kMaxUrlChunks; ++i) {
-      entries.push_back(google_breakpad::CustomInfoEntry(
-          StringPrintf(L"url-chunk-%i", i + 1).c_str(), L""));
-      (*g_url_chunks)[i] = entries.back().value;
-    }
-  } else {
-    // Browser-specific entries.
-    google_breakpad::CustomInfoEntry switch1(L"switch-1", L"");
-    google_breakpad::CustomInfoEntry switch2(L"switch-2", L"");
-
-    // Get the first two command line switches if they exist. The CommandLine
-    // class does not allow to enumerate the switches so we do it by hand.
-    int num_args = 0;
-    wchar_t** args = ::CommandLineToArgvW(::GetCommandLineW(), &num_args);
-    if (args) {
-      if (num_args > 1)
-        switch1.set_value(TrimToBreakpadMax(args[1]).c_str());
-      if (num_args > 2)
-        switch2.set_value(TrimToBreakpadMax(args[2]).c_str());
-    }
-
-    entries.push_back(switch1);
-    entries.push_back(switch2);
+    google_breakpad::CustomInfoEntry url1(L"url-chunk-1", L"");
+    google_breakpad::CustomInfoEntry url2(L"url-chunk-2", L"");
+    google_breakpad::CustomInfoEntry url3(L"url-chunk-3", L"");
+    google_breakpad::CustomInfoEntry url4(L"url-chunk-4", L"");
+    google_breakpad::CustomInfoEntry url5(L"url-chunk-5", L"");
+    google_breakpad::CustomInfoEntry url6(L"url-chunk-6", L"");
+    google_breakpad::CustomInfoEntry url7(L"url-chunk-7", L"");
+    google_breakpad::CustomInfoEntry url8(L"url-chunk-8", L"");
+
+    static google_breakpad::CustomInfoEntry entries[] =
+        { ver_entry, prod_entry, plat_entry, type_entry, guid_entry,
+          url1, url2, url3, url4, url5, url6, url7, url8 };
+
+    std::vector<wchar_t*>* tmp_url_chunks = new std::vector<wchar_t*>(8);
+    for (size_t i = 0; i < 8; ++i)
+      (*tmp_url_chunks)[i] = entries[5 + i].value;
+    g_url_chunks = tmp_url_chunks;
+
+    g_client_id = entries[4].value;
+
+    static google_breakpad::CustomClientInfo custom_info_renderer
+        = {entries, arraysize(entries)};
+    return &custom_info_renderer;
   }
 
-  // If this fails, kNumCustomInfoEntries needs to be increased.
-  DCHECK(entries.size() <= entries.capacity());
+  // Browser-specific entries.
+  google_breakpad::CustomInfoEntry switch1(L"switch-1", L"");
+  google_breakpad::CustomInfoEntry switch2(L"switch-2", L"");
+
+  // Get the first two command line switches if they exist. The CommandLine
+  // class does not allow to enumerate the switches so we do it by hand.
+  int num_args = 0;
+  wchar_t** args = ::CommandLineToArgvW(::GetCommandLineW(), &num_args);
+  if (args) {
+    if (num_args > 1)
+      switch1.set_value(TrimToBreakpadMax(args[1]).c_str());
+    if (num_args > 2)
+      switch2.set_value(TrimToBreakpadMax(args[2]).c_str());
+  }
 
+  static google_breakpad::CustomInfoEntry entries[] =
+      {ver_entry, prod_entry, plat_entry, type_entry, guid_entry,
+       switch1, switch2};
+  g_client_id = entries[4].value;
   static google_breakpad::CustomClientInfo custom_info_browser =
-      {&entries.front(), entries.size()};
-
+      {entries, arraysize(entries)};
   return &custom_info_browser;
 }
 
@@ -249,19 +249,6 @@ extern "C" void __declspec(dllexport) __cdecl SetClientId(
            client_id);
 }
 
-extern "C" void __declspec(dllexport) __cdecl SetExtensionID(
-    int index, const wchar_t* id) {
-  DCHECK(id);
-  DCHECK(index < kMaxReportedActiveExtensions);
-
-  if (!g_extension_ids)
-    return;
-
-  wcscpy_s((*g_extension_ids)[index], 
-           google_breakpad::CustomInfoEntry::kValueMaxLength,
-           id);
-}
-
 }  // namespace
 
 // This function is executed by the child process that DumpDoneCallback()
diff --git a/chrome/app/breakpad_win.h b/chrome/app/breakpad_win.h
index 3a13149..c7cd9ec 100644
--- a/chrome/app/breakpad_win.h
+++ b/chrome/app/breakpad_win.h
@@ -8,12 +8,6 @@
 #include <windows.h>
 #include <string>
 
-// The maximum number of 64-char URL chunks we will report.
-static const int kMaxUrlChunks = 8;
-
-// The maximum number of active extensions we will report.
-static const int kMaxReportedActiveExtensions = 10;
-
 // Calls InitCrashReporterThread in it's own thread for the browser process
 // or directly for the plugin and renderer process.
 void InitCrashReporterWithDllPath(const std::wstring& dll_path);
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 078b1b7..c254b92 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -24,7 +24,6 @@
 #include "chrome/browser/extensions/external_pref_extension_provider.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
-#include "chrome/common/child_process_logging.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
@@ -410,8 +409,6 @@ void ExtensionsService::NotifyExtensionLoaded(Extension* extension) {
 void ExtensionsService::NotifyExtensionUnloaded(Extension* extension) {
   LOG(INFO) << "Sending EXTENSION_UNLOADED";
 
-  UpdateActiveExtensionsInCrashReporter();
-
   NotificationService::current()->Notify(
       NotificationType::EXTENSION_UNLOADED,
       Source<Profile>(profile_),
@@ -619,16 +616,6 @@ void ExtensionsService::OnExtensionLoaded(Extension* extension,
         break;
     }
   }
-
-  UpdateActiveExtensionsInCrashReporter();
-}
-
-void ExtensionsService::UpdateActiveExtensionsInCrashReporter() {
-  std::vector<std::string> extension_ids;
-  for (size_t i = 0; i < extensions_.size(); ++i)
-    extension_ids.push_back(extensions_[i]->id());
-
-  child_process_logging::SetActiveExtensions(extension_ids);
 }
 
 void ExtensionsService::OnExtensionInstalled(Extension* extension,
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index b848d0b..4a91fff 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -253,9 +253,6 @@ class ExtensionsService
   // Handles sending notification that |extension| was unloaded.
   void NotifyExtensionUnloaded(Extension* extension);
 
-  // Helper that updates the active extension list used for crash reporting.
-  void UpdateActiveExtensionsInCrashReporter();
-
   // The profile this ExtensionsService is part of.
   Profile* profile_;
 
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index 534991a..31736f9 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -584,14 +584,6 @@ void BrowserRenderProcessHost::InitExtensions() {
 
 void BrowserRenderProcessHost::SendUserScriptsUpdate(
     base::SharedMemory *shared_memory) {
-  // Don't send user scripts to extension processes. We currently don't allow
-  // user scripts to run in extensions, so it would be pointless. It would also
-  // mess up the crash reporting, which sends a different set of "active"
-  // extensions depending on whether the process is an extension or renderer
-  // process.
-  if (extension_process_)
-    return;
-
   // Process is being started asynchronously.  We'll end up calling
   // InitUserScripts when it's created which will call this again.
   if (child_process_.get() && child_process_->IsStarting())
diff --git a/chrome/common/child_process_logging.h b/chrome/common/child_process_logging.h
index 513359f..aa336af 100644
--- a/chrome/common/child_process_logging.h
+++ b/chrome/common/child_process_logging.h
@@ -5,8 +5,6 @@
 #ifndef CHROME_COMMON_CHILD_PROCESS_LOGGING_H_
 #define CHROME_COMMON_CHILD_PROCESS_LOGGING_H_
 
-#include <vector>
-
 #include "base/basictypes.h"
 #include "googleurl/src/gurl.h"
 
@@ -19,14 +17,6 @@ void SetActiveURL(const GURL& url);
 // Sets the Client ID that is used as GUID if a Chrome process crashes.
 void SetClientId(const std::string& client_id);
 
-// Sets the list of "active" extensions in this process. We overload "active" to
-// mean different things depending on the process type:
-// - browser: all enabled extensions
-// - renderer: the unique set of extension ids from all content scripts
-// - extension: the id of each extension running in this process (there can be
-//   multiple because of process collapsing).
-void SetActiveExtensions(const std::vector<std::string>& extension_ids);
-
 // Simple wrapper class that sets the active URL in it's constructor and clears
 // the active URL in the destructor.
 class ScopedActiveURLSetter {
diff --git a/chrome/common/child_process_logging_linux.cc b/chrome/common/child_process_logging_linux.cc
index 0b54010..9b51303 100644
--- a/chrome/common/child_process_logging_linux.cc
+++ b/chrome/common/child_process_logging_linux.cc
@@ -31,8 +31,4 @@ void SetClientId(const std::string& client_id) {
   std::wstring wstr = ASCIIToWide(str);
   GoogleUpdateSettings::SetMetricsId(wstr);
 }
-
-void SetActiveExtensions(const std::vector<std::string> extension_ids) {
-  // TODO(port)
-}
 }  // namespace child_process_logging
diff --git a/chrome/common/child_process_logging_mac.mm b/chrome/common/child_process_logging_mac.mm
index 8ed617d..9e89c01 100644
--- a/chrome/common/child_process_logging_mac.mm
+++ b/chrome/common/child_process_logging_mac.mm
@@ -91,9 +91,4 @@ void SetClientId(const std::string& client_id) {
   std::wstring wstr = ASCIIToWide(str);
   GoogleUpdateSettings::SetMetricsId(wstr);
 }
-
-void SetActiveExtensions(const std::vector<std::string> extension_ids) {
-  // TODO(port)
-}
-
 }  // namespace child_process_logging
diff --git a/chrome/common/child_process_logging_win.cc b/chrome/common/child_process_logging_win.cc
index 9a11e8f..94b77d8 100644
--- a/chrome/common/child_process_logging_win.cc
+++ b/chrome/common/child_process_logging_win.cc
@@ -7,7 +7,6 @@
 #include <windows.h>
 
 #include "base/string_util.h"
-#include "chrome/app/breakpad_win.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "googleurl/src/gurl.h"
@@ -19,10 +18,6 @@ typedef void (__cdecl *MainSetActiveURL)(const wchar_t*);
 // exported in breakpad_win.cc: void __declspec(dllexport) __cdecl SetClientId.
 typedef void (__cdecl *MainSetClientId)(const wchar_t*);
 
-// exported in breakpad_win.cc:
-// void __declspec(dllexport) __cdecl SetExtensionID.
-typedef void (__cdecl *MainSetExtensionID)(size_t, const wchar_t*);
-
 void SetActiveURL(const GURL& url) {
   static MainSetActiveURL set_active_url = NULL;
   // note: benign race condition on set_active_url.
@@ -66,24 +61,4 @@ void SetClientId(const std::string& client_id) {
   (set_client_id)(wstr.c_str());
 }
 
-void SetActiveExtensions(const std::vector<std::string>& extension_ids) {
-  static MainSetExtensionID set_extension_id = NULL;
-  if (!set_extension_id) {
-    HMODULE exe_module = GetModuleHandle(chrome::kBrowserProcessExecutableName);
-    if (!exe_module)
-      return;
-    set_extension_id = reinterpret_cast<MainSetExtensionID>(
-        GetProcAddress(exe_module, "SetExtensionID"));
-    if (!set_extension_id)
-      return;
-  }
-
-  for (size_t i = 0; i < kMaxReportedActiveExtensions; ++i) {
-    if (i < extension_ids.size())
-      (set_extension_id)(i, ASCIIToWide(extension_ids[i].c_str()).c_str());
-    else
-      (set_extension_id)(i, L"");
-  }
-}
-
 }  // namespace child_process_logging
diff --git a/chrome/renderer/extensions/extension_process_bindings.cc b/chrome/renderer/extensions/extension_process_bindings.cc
index a2b28cd..37cb260 100644
--- a/chrome/renderer/extensions/extension_process_bindings.cc
+++ b/chrome/renderer/extensions/extension_process_bindings.cc
@@ -9,11 +9,8 @@
 #include <string>
 #include <vector>
 
-#include "base/command_line.h"
 #include "base/json/json_reader.h"
 #include "base/singleton.h"
-#include "chrome/common/child_process_logging.h"
-#include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_message_bundle.h"
 #include "chrome/common/extensions/url_pattern.h"
@@ -105,19 +102,6 @@ static L10nMessagesMap* GetL10nMessagesMap(const std::string extension_id) {
   }
 }
 
-static std::vector<std::string> GetActiveExtensionIDs() {
-  std::vector<std::string> extension_ids;
-  ExtensionPermissionsMap& permissions =
-      Singleton<SingletonData>()->permissions_;
-
-  for (ExtensionPermissionsMap::iterator iter = permissions.begin();
-       iter != permissions.end(); ++iter) {
-    extension_ids.push_back(iter->first);
-  }
-
-  return extension_ids;
-}
-
 // A RenderViewVisitor class that iterates through the set of available
 // views, looking for a view of the given type, in the given browser window
 // and within the given extension.
@@ -642,12 +626,6 @@ void ExtensionProcessBindings::SetAPIPermissions(
     permissions_map[Extension::kPermissionNames[i]] = false;
   for (size_t i = 0; i < permissions.size(); ++i)
     permissions_map[permissions[i]] = true;
-
-  // Ugly hack. We also update our list of active extensions here. This always
-  // gets called, even if the extension has no api permissions. In single
-  // process, this has already been done in the browser code.
-  if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess))
-    child_process_logging::SetActiveExtensions(GetActiveExtensionIDs());
 }
 
 // static
diff --git a/chrome/renderer/user_script_slave.cc b/chrome/renderer/user_script_slave.cc
index 4820c83..1133849 100644
--- a/chrome/renderer/user_script_slave.cc
+++ b/chrome/renderer/user_script_slave.cc
@@ -5,15 +5,12 @@
 #include "chrome/renderer/user_script_slave.h"
 
 #include "app/resource_bundle.h"
-#include "base/command_line.h"
 #include "base/histogram.h"
 #include "base/logging.h"
 #include "base/perftimer.h"
 #include "base/pickle.h"
 #include "base/shared_memory.h"
 #include "base/string_util.h"
-#include "chrome/common/child_process_logging.h"
-#include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/renderer/extension_groups.h"
@@ -114,24 +111,6 @@ bool UserScriptSlave::UpdateScripts(base::SharedMemoryHandle shared_memory) {
     }
   }
 
-  // Update the crash reporter with all loaded extensions. In single process,
-  // this has already been done in the browser code.
-  if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess)) {
-    std::vector<std::string> extension_ids;
-    for (size_t i = 0; i < num_scripts; ++i) {
-      DCHECK(!scripts_[i]->extension_id().empty());
-
-      // We must check this because there can be multiple scripts from a single
-      // extension. n^2, but meh, it's a small list.
-      if (std::find(extension_ids.begin(), extension_ids.end(),
-                    scripts_[i]->extension_id()) == extension_ids.end()) {
-        extension_ids.push_back(scripts_[i]->extension_id());
-      }
-    }
-
-    child_process_logging::SetActiveExtensions(extension_ids);
-  }
-
   return true;
 }