Merge 33541 - Ok, here is a different approach at this change.

Instead of storing pointers into a vector, store offsets. That
way, if the vector resizes, we should still be OK.

Also, add a DCHECK that we only enter this method once, which
is my current assumption.

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

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

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

11 files changed, 194 insertions, 65 deletions

diff --git a/chrome/app/breakpad_win.cc b/chrome/app/breakpad_win.cc
index 297a034..2db9224 100644
--- a/chrome/app/breakpad_win.cc
+++ b/chrome/app/breakpad_win.cc
@@ -33,11 +33,13 @@ const wchar_t kSystemPrincipalSid[] =L"S-1-5-18";
 
 google_breakpad::ExceptionHandler* g_breakpad = NULL;
 
-std::vector<wchar_t*>* g_url_chunks = NULL;
-
-// A string containing the user's unique metric services id. We send this
-// in the crash report.
-wchar_t* g_client_id = NULL;
+// A pointer to the custom entries that we send in the event of a crash. We need
+// this pointer, along with the offsets into it below, so that we can keep the
+// data updated as the state of the browser changes.
+static std::vector<google_breakpad::CustomInfoEntry>* g_custom_entries = NULL;
+static size_t g_url_chunks_offset;
+static size_t g_extension_ids_offset;
+static size_t g_client_id_offset;
 
 // Dumps the current process memory.
 extern "C" void __declspec(dllexport) __cdecl DumpProcess() {
@@ -74,70 +76,67 @@ google_breakpad::CustomClientInfo* GetCustomInfo(const std::wstring& dll_path,
      version = L"0.0.0.0-devel";
   }
 
-  // Common entries.
-  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());
+  // We only expect this method to be called once per process.
+  DCHECK(!g_custom_entries);
+  g_custom_entries = new std::vector<google_breakpad::CustomInfoEntry>;
+
+  // Common g_custom_entries.
+  g_custom_entries->push_back(
+      google_breakpad::CustomInfoEntry(L"ver", version.c_str()));
+  g_custom_entries->push_back(
+      google_breakpad::CustomInfoEntry(L"plat", L"Win32"));
+  g_custom_entries->push_back(
+      google_breakpad::CustomInfoEntry(L"ptype", type.c_str()));
+
+  g_extension_ids_offset = g_custom_entries->size();
+  for (int i = 0; i < kMaxReportedActiveExtensions; ++i) {
+    g_custom_entries->push_back(google_breakpad::CustomInfoEntry(
+        StringPrintf(L"extension-%i", i + 1).c_str(), L""));
+  }
 
   // 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);
-  google_breakpad::CustomInfoEntry guid_entry(L"guid", guid.c_str());
+  g_client_id_offset = g_custom_entries->size();
+  g_custom_entries->push_back(
+      google_breakpad::CustomInfoEntry(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.
-    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;
-  }
+    // and split the URL across the g_custom_entries.
+    g_url_chunks_offset = g_custom_entries->size();
+    for (int i = 0; i < kMaxUrlChunks; ++i) {
+      g_custom_entries->push_back(google_breakpad::CustomInfoEntry(
+          StringPrintf(L"url-chunk-%i", i + 1).c_str(), L""));
+    }
+  } else {
+    // Browser-specific g_custom_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());
+    }
 
-  // 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());
+    g_custom_entries->push_back(switch1);
+    g_custom_entries->push_back(switch2);
   }
 
-  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, arraysize(entries)};
-  return &custom_info_browser;
+  static google_breakpad::CustomClientInfo custom_client_info;
+  custom_client_info.entries = &g_custom_entries->front();
+  custom_client_info.count = g_custom_entries->size();
+
+  return &custom_client_info;
 }
 
 // Contains the information needed by the worker thread.
@@ -213,30 +212,30 @@ long WINAPI ChromeExceptionFilter(EXCEPTION_POINTERS* info) {
 extern "C" void __declspec(dllexport) __cdecl SetActiveURL(
     const wchar_t* url_cstring) {
   DCHECK(url_cstring);
-  if (!g_url_chunks)
-    return;
 
   std::wstring url(url_cstring);
-  size_t num_chunks = g_url_chunks->size();
   size_t chunk_index = 0;
   size_t url_size = url.size();
 
   // Split the url across all the chunks.
   for (size_t url_offset = 0;
-       chunk_index < num_chunks && url_offset < url_size; ++chunk_index) {
+       chunk_index < kMaxUrlChunks && url_offset < url_size; ++chunk_index) {
     size_t current_chunk_size = std::min(url_size - url_offset,
         static_cast<size_t>(
             google_breakpad::CustomInfoEntry::kValueMaxLength - 1));
-    url._Copy_s((*g_url_chunks)[chunk_index],
+
+    wchar_t* entry_value =
+        (*g_custom_entries)[g_url_chunks_offset + chunk_index].value;
+    url._Copy_s(entry_value,
                 google_breakpad::CustomInfoEntry::kValueMaxLength,
                 current_chunk_size, url_offset);
-    (*g_url_chunks)[chunk_index][current_chunk_size] = L'\0';
+    entry_value[current_chunk_size] = L'\0';
     url_offset += current_chunk_size;
   }
 
   // And null terminate any unneeded chunks.
-  for (; chunk_index < num_chunks; ++chunk_index)
-    (*g_url_chunks)[chunk_index][0] = L'\0';
+  for (; chunk_index < kMaxUrlChunks; ++chunk_index)
+    (*g_custom_entries)[g_url_chunks_offset + chunk_index].value[0] = L'\0';
 }
 
 extern "C" void __declspec(dllexport) __cdecl SetClientId(
@@ -244,11 +243,21 @@ extern "C" void __declspec(dllexport) __cdecl SetClientId(
   if (client_id == NULL)
     return;
 
-  wcscpy_s(g_client_id,
+  wcscpy_s((*g_custom_entries)[g_client_id_offset].value,
            google_breakpad::CustomInfoEntry::kValueMaxLength,
            client_id);
 }
 
+extern "C" void __declspec(dllexport) __cdecl SetExtensionID(
+    int index, const wchar_t* id) {
+  DCHECK(id);
+  DCHECK(index < kMaxReportedActiveExtensions);
+
+  wcscpy_s((*g_custom_entries)[g_extension_ids_offset + index].value, 
+           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 c7cd9ec..3a13149 100644
--- a/chrome/app/breakpad_win.h
+++ b/chrome/app/breakpad_win.h
@@ -8,6 +8,12 @@
 #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 7e66ee3..867fde5 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -24,6 +24,7 @@
 #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"
@@ -261,6 +262,7 @@ void ExtensionsService::EnableExtension(const std::string& extension_id) {
       extension->GetChromeURLOverrides());
 
   NotifyExtensionLoaded(extension);
+  UpdateActiveExtensionsInCrashReporter();
 }
 
 void ExtensionsService::DisableExtension(const std::string& extension_id) {
@@ -285,6 +287,7 @@ void ExtensionsService::DisableExtension(const std::string& extension_id) {
       extension->GetChromeURLOverrides());
 
   NotifyExtensionUnloaded(extension);
+  UpdateActiveExtensionsInCrashReporter();
 }
 
 void ExtensionsService::LoadExtension(const FilePath& extension_path) {
@@ -503,6 +506,7 @@ void ExtensionsService::UnloadExtension(const std::string& extension_id) {
   extensions_.erase(iter);
 
   NotifyExtensionUnloaded(extension.get());
+  UpdateActiveExtensionsInCrashReporter();
 }
 
 void ExtensionsService::UnloadAllExtensions() {
@@ -622,6 +626,18 @@ 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) {
+    if (!extensions_[i]->IsTheme())
+      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 4a91fff..b848d0b 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -253,6 +253,9 @@ 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 31736f9..534991a 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -584,6 +584,14 @@ 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 aa336af..513359f 100644
--- a/chrome/common/child_process_logging.h
+++ b/chrome/common/child_process_logging.h
@@ -5,6 +5,8 @@
 #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"
 
@@ -17,6 +19,14 @@ 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 9b51303..bbe18fd 100644
--- a/chrome/common/child_process_logging_linux.cc
+++ b/chrome/common/child_process_logging_linux.cc
@@ -31,4 +31,8 @@ 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 9e89c01..5d46e01 100644
--- a/chrome/common/child_process_logging_mac.mm
+++ b/chrome/common/child_process_logging_mac.mm
@@ -91,4 +91,9 @@ 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 94b77d8..9a11e8f 100644
--- a/chrome/common/child_process_logging_win.cc
+++ b/chrome/common/child_process_logging_win.cc
@@ -7,6 +7,7 @@
 #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"
@@ -18,6 +19,10 @@ 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.
@@ -61,4 +66,24 @@ 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 37cb260..a2b28cd 100644
--- a/chrome/renderer/extensions/extension_process_bindings.cc
+++ b/chrome/renderer/extensions/extension_process_bindings.cc
@@ -9,8 +9,11 @@
 #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"
@@ -102,6 +105,19 @@ 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.
@@ -626,6 +642,12 @@ 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 1133849..4820c83 100644
--- a/chrome/renderer/user_script_slave.cc
+++ b/chrome/renderer/user_script_slave.cc
@@ -5,12 +5,15 @@
 #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"
@@ -111,6 +114,24 @@ 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;
 }