Revert debugging crud I added while trying to track down bug 31719.

Revert "Speculative fix for the windows theme crasher." (r36036)
Revert "More debugging statements to try to track down BrowserThemePack crash." (r37259).

BUG=31719
TEST=none

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

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

3 files changed, 16 insertions, 105 deletions

diff --git a/chrome/browser/browser_theme_pack.cc b/chrome/browser/browser_theme_pack.cc
index ed45ad0..9f54abc 100644
--- a/chrome/browser/browser_theme_pack.cc
+++ b/chrome/browser/browser_theme_pack.cc
@@ -26,12 +26,6 @@
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkUnPreMultiply.h"
 
-// No optimizations under windows until we know what's up with the crashing.
-#if defined(OS_WIN)
-#pragma optimize("", off)
-#pragma warning(disable:4748)
-#endif
-
 namespace {
 
 // Version number of the current theme pack. We just throw out and rebuild
@@ -353,8 +347,8 @@ BrowserThemePack::~BrowserThemePack() {
 // static
 BrowserThemePack* BrowserThemePack::BuildFromExtension(Extension* extension) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-  CHECK(extension);
-  CHECK(extension->IsTheme());
+  DCHECK(extension);
+  DCHECK(extension->IsTheme());
 
   BrowserThemePack* pack = new BrowserThemePack;
   pack->BuildHeader(extension);
@@ -443,18 +437,7 @@ scoped_refptr<BrowserThemePack> BrowserThemePack::BuildFromDataPack(
 }
 
 bool BrowserThemePack::WriteToDisk(FilePath path) const {
-  FilePath::CharType full_path_on_stack[512 + 1];
-  int image_memory_size = image_memory_.size();
-  int prepared_images_size = prepared_images_.size();
-
-  // Copy path's backing string onto the stack because that's what get's stored
-  // in minidumps. :(
-  size_t i = 0;
-  for (i = 0; i < 512 && i < path.value().size(); ++i) {
-    full_path_on_stack[i] = path.value()[i];
-  }
-  full_path_on_stack[i] = '\0';
-
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
   // Add resources for each of the property arrays.
   RawDataForWriting resources;
   resources[kHeaderID] = base::StringPiece(
@@ -482,11 +465,6 @@ bool BrowserThemePack::WriteToDisk(FilePath path) const {
   RepackImages(prepared_images_, &reencoded_images);
   AddRawImagesTo(reencoded_images, &resources);
 
-  // Force the values to stick around since the crash happens in
-  // reencoded_images.
-  LOG(INFO) << full_path_on_stack << " / " << image_memory_size
-            << prepared_images_size;
-
   return base::DataPack::WritePack(path, resources);
 }
 
@@ -1030,6 +1008,7 @@ void BrowserThemePack::GenerateTabBackgroundImages(ImageCache* bitmaps) const {
 
 void BrowserThemePack::RepackImages(const ImageCache& images,
                                     RawImages* reencoded_images) const {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
   for (ImageCache::const_iterator it = images.begin();
        it != images.end(); ++it) {
     std::vector<unsigned char> image_data;
@@ -1037,16 +1016,14 @@ void BrowserThemePack::RepackImages(const ImageCache& images,
       NOTREACHED() << "Image file for resource " << it->first
                    << " could not be encoded.";
     } else {
-      RefCountedBytes* bytes = RefCountedBytes::TakeVector(&image_data);
-      CHECK(bytes);
-      (*reencoded_images)[it->first] = bytes;
+      (*reencoded_images)[it->first] = RefCountedBytes::TakeVector(&image_data);
     }
   }
 }
 
 void BrowserThemePack::MergeImageCaches(
     const ImageCache& source, ImageCache* destination) const {
-  CHECK(destination);
+
   for (ImageCache::const_iterator it = source.begin(); it != source.end();
        ++it) {
     ImageCache::const_iterator bitmap_it = destination->find(it->first);
@@ -1059,16 +1036,9 @@ void BrowserThemePack::MergeImageCaches(
 
 void BrowserThemePack::AddRawImagesTo(const RawImages& images,
                                       RawDataForWriting* out) const {
-  int resource_at_time_of_crash = -1;
-
-  CHECK(out);
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
   for (RawImages::const_iterator it = images.begin(); it != images.end();
        ++it) {
-    resource_at_time_of_crash = it->first;
-
-    CHECK(it->second->front());
-    CHECK(it->second->size());
-
     (*out)[it->first] = base::StringPiece(
         reinterpret_cast<const char*>(it->second->front()), it->second->size());
   }
@@ -1089,9 +1059,3 @@ color_utils::HSL BrowserThemePack::GetTintInternal(int id) const {
 
   return BrowserThemeProvider::GetDefaultTint(id);
 }
-
-// No optimizations under windows until we know what's up with the crashing.
-#if defined(OS_WIN)
-#pragma warning(default:4748)
-#pragma optimize("", on)
-#endif
diff --git a/chrome/browser/browser_theme_provider.cc b/chrome/browser/browser_theme_provider.cc
index acbda59..dd2f301 100644
--- a/chrome/browser/browser_theme_provider.cc
+++ b/chrome/browser/browser_theme_provider.cc
@@ -38,12 +38,6 @@
 #include "app/win_util.h"
 #endif
 
-// No optimizations under windows until we know what's up with the crashing.
-#if defined(OS_WIN)
-#pragma optimize("", off)
-#pragma warning(disable:4748)
-#endif
-
 // Strings used in alignment properties.
 const char* BrowserThemeProvider::kAlignmentTop = "top";
 const char* BrowserThemeProvider::kAlignmentBottom = "bottom";
@@ -279,7 +273,7 @@ void BrowserThemeProvider::SetTheme(Extension* extension) {
   DCHECK(extension);
   DCHECK(extension->IsTheme());
 
-  BuildFromExtension(extension, false);
+  BuildFromExtension(extension);
   SaveThemeID(extension->id());
 
   NotifyThemeChanged();
@@ -528,7 +522,9 @@ void BrowserThemeProvider::LoadThemePrefs() {
       if (service) {
         Extension* extension = service->GetExtensionById(current_id, false);
         if (extension) {
-          MigrateTheme(extension, extension->name());
+          DLOG(ERROR) << "Migrating theme";
+          BuildFromExtension(extension);
+          UserMetrics::RecordAction("Themes.Migrated", profile_);
         } else {
           DLOG(ERROR) << "Theme is mysteriously gone.";
           ClearAllThemeData();
@@ -562,31 +558,7 @@ void BrowserThemeProvider::SaveThemeID(const std::string& id) {
   profile_->GetPrefs()->SetString(prefs::kCurrentThemeID, UTF8ToWide(id));
 }
 
-void BrowserThemeProvider::MigrateTheme(Extension* extension,
-                                        const std::string& name) {
-  FilePath::CharType full_name_on_stack[512 + 1];
-
-  // Copy names's backing string onto the stack because that's what get's
-  // stored in minidumps. :(
-  size_t i = 0;
-  for (i = 0; i < 512 && i < name.size(); ++i) {
-    full_name_on_stack[i] = name[i];
-  }
-  full_name_on_stack[i] = '\0';
-
-  // TODO(erg): Remove this hack.
-  //
-  // This is a hack to force the name of the theme into the stack
-  // frame. Hopefully.
-  BuildFromExtension(extension, true);
-  UserMetrics::RecordAction("Themes.Migrated", profile_);
-  LOG(INFO) << "Migrating theme: " << full_name_on_stack;
-}
-
-void BrowserThemeProvider::BuildFromExtension(Extension* extension,
-                                              bool synchronously) {
-  CHECK(extension);
-
+void BrowserThemeProvider::BuildFromExtension(Extension* extension) {
   scoped_refptr<BrowserThemePack> pack =
       BrowserThemePack::BuildFromExtension(extension);
   if (!pack.get()) {
@@ -598,16 +570,8 @@ void BrowserThemeProvider::BuildFromExtension(Extension* extension,
 
   // Write the packed file to disk.
   FilePath pack_path = extension->path().Append(chrome::kThemePackFilename);
-  if (synchronously) {
-    // Write to disk sychronously because we don't have a message loop yet.
-    if (!pack->WriteToDisk(pack_path)) {
-      NOTREACHED() << "Could not write theme pack to disk";
-    }
-  } else {
-    // Post a task to write to disk since we have a message loop to post from.
-    ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
-                           new WritePackToDiskTask(pack, pack_path));
-  }
+  ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
+                         new WritePackToDiskTask(pack, pack_path));
 
   SavePackName(pack_path);
   theme_pack_ = pack;
@@ -623,9 +587,3 @@ void BrowserThemeProvider::OnInfobarDestroyed() {
   if (number_of_infobars_ == 0)
     RemoveUnusedThemes();
 }
-
-// No optimizations under windows until we know what's up with the crashing.
-#if defined(OS_WIN)
-#pragma warning(default:4748)
-#pragma optimize("", on)
-#endif
diff --git a/chrome/browser/browser_theme_provider.h b/chrome/browser/browser_theme_provider.h
index 70f9d57..1474c24 100644
--- a/chrome/browser/browser_theme_provider.h
+++ b/chrome/browser/browser_theme_provider.h
@@ -197,20 +197,9 @@ class BrowserThemeProvider : public NonThreadSafe,
   // Save the id of the last theme installed.
   void SaveThemeID(const std::string& id);
 
-  // A temporary hack to force |name| onto the stack to see if a
-  // certain theme is causing all these crashes in case my speculative
-  // fix doesn't do it.
-  void MigrateTheme(Extension* extension, const std::string& name);
-
   // Implementation of SetTheme() (and the fallback from LoadThemePrefs() in
-  // case we don't have a theme pack). We specify |synchronously| because of
-  // a potential data race. BuildFromExtension() is called during startup and
-  // will be called before we have a message loop instantiated on the main
-  // UI thread. Therefore, we can't post tasks FROM the UI thread. We'll work
-  // synchronously during theme upgrades and missing theme paks, and will shunt
-  // encoding and disk writing work onto the other thread when we can't post
-  // tasks.
-  void BuildFromExtension(Extension* extension, bool synchronously);
+  // case we don't have a theme pack).
+  void BuildFromExtension(Extension* extension);
 
   // Remove preference values for themes that are no longer in use.
   void RemoveUnusedThemes();