Replacing base::DIR_TEMP with ScopedTempDir when appropriate.

chrome/browser/extensions/extension_browsertest.cc

 // 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.
 
 #include "chrome/browser/extensions/extension_browsertest.h"
 
 #include <vector>
 
 #include "base/command_line.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/string_number_conversions.h"
+#include "base/memory/scoped_temp_dir.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_creator.h"
 #include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/extensions/extension_install_ui.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/omnibox/location_bar.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/ui_test_utils.h"
 #include "content/common/notification_registrar.h"
 #include "content/common/notification_service.h"
 #include "content/common/notification_type.h"
 
 ExtensionBrowserTest::ExtensionBrowserTest()
     : loaded_(false),
       installed_(false),
       extension_installs_observed_(0),
       target_page_action_count_(-1),
       target_visible_page_action_count_(-1) {
+  DCHECK(temp_dir_.CreateUniqueTempDir());

[Paweł Hajdan Jr., 2011/04/11 18:16:53]

Oops, two things are wrong about that:

1) DCHECK only runs in Debug mode. The temporary directory will not be created
in Release mode.

2) We avoid DCHECK in tests because they abort the entire process. Instead, you
should be able to just use EXPECT_TRUE. Also, it's probably better to put it in
something SetUp-like rather than the ctor.

[Mike West, 2011/04/12 07:04:04]

Right, I kinda knew that DCHECK was wrong, but I didn't know about EXPECT_TRUE. 
Thanks for the tip.

With regard to putting the check in the ctor, I agree that it's a bit rough, but
I don't know this class well enough to know what gets called in what order.
Putting it in the constructor ensured that it got set up once and only once
before any of the methods that end up calling `PackExtension`.

Adding a `SetUp` method is certainly viable, but then I'd just be calling that
from the constructor. I'm not sure that's any better.

[Paweł Hajdan Jr., 2011/04/12 08:32:02]

EXPECT_TRUE in ctor is fine for me. BrowserTest has a tricky initialization,
lots of SetUp-like methods.

 }
 
 void ExtensionBrowserTest::SetUpCommandLine(CommandLine* command_line) {
   // This enables DOM automation for tab contentses.
   EnableDOMAutomation();
 
   // This enables it for extension hosts.
   ExtensionHost::EnableDOMAutomation();
 
   PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir_);
@@ skipping 72 matching lines @@
                                    &manifest))
     return false;
 
   service->LoadComponentExtension(
       ExtensionService::ComponentExtensionInfo(manifest, path));
 
   return true;
 }
 
 FilePath ExtensionBrowserTest::PackExtension(const FilePath& dir_path) {
-  FilePath crx_path;
-  if (!PathService::Get(base::DIR_TEMP, &crx_path)) {
-    ADD_FAILURE() << "Failed to get DIR_TEMP from PathService.";
-    return FilePath();
-  }
-  crx_path = crx_path.AppendASCII("temp.crx");
+  FilePath crx_path = temp_dir_.path().AppendASCII("temp.crx");
   if (!file_util::Delete(crx_path, false)) {
     ADD_FAILURE() << "Failed to delete crx: " << crx_path.value();
     return FilePath();
   }
 
   FilePath pem_path = crx_path.DirName().AppendASCII("temp.pem");
   if (!file_util::Delete(pem_path, false)) {
     ADD_FAILURE() << "Failed to delete pem: " << pem_path.value();
     return FilePath();
   }
@@ skipping 296 matching lines @@
         MessageLoopForUI::current()->Quit();
       }
       break;
     }
 
     default:
       NOTREACHED();
       break;
   }
 }