Using ScopedTempDir as the download directory used by the DownloadViaPost test.

chrome/browser/extensions/api/extension_action/browser_action_apitest.cc

Index: chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
diff --git a/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc b/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
index f02bb13d2bc6e7160b3690990cbce4dd68a433e9..22b8619b30e1a2b3b67155a154e65d3e3bc771d4 100644
--- a/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
+++ b/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
@@ -4,6 +4,7 @@
 
 #include <stdint.h>
 
+#include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
@@ -23,8 +24,10 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/extensions/extension_process_policy.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_frame_host.h"
@@ -851,6 +854,25 @@ class NavigatingExtensionPopupBrowserTest : public BrowserActionApiTest {
                         navigation_starting_script);
   }
 
+  // Must be called after browser creation.  Creates a temporary
+  // directory for downloads that is auto-deleted on destruction.
+  // Returning false indicates a failure of the function, and should be asserted
+  // in the caller.
+  bool CreateAndSetDownloadsDirectory(Browser* browser) {
+    if (!browser)
+      return false;
+
+    if (!downloads_directory_.CreateUniqueTempDir())
+      return false;
+
+    browser->profile()->GetPrefs()->SetFilePath(
+        prefs::kDownloadDefaultDirectory, downloads_directory_.GetPath());
+    browser->profile()->GetPrefs()->SetFilePath(
+        prefs::kSaveFileDefaultDirectory, downloads_directory_.GetPath());
+
+    return true;
+  }
+
  private:
   void TestPopupNavigation(const GURL& target_url,
                            ExpectedNavigationStatus expected_navigation_status,
@@ -925,6 +947,7 @@ class NavigatingExtensionPopupBrowserTest : public BrowserActionApiTest {
 
   const Extension* popup_extension_;
   const Extension* other_extension_;
+  base::ScopedTempDir downloads_directory_;
 };
 
 // Tests that an extension pop-up cannot be navigated to a web page.
@@ -991,16 +1014,8 @@ IN_PROC_BROWSER_TEST_F(NavigatingExtensionPopupBrowserTest,
 // GET should automagically start working for downloads.
 // TODO(lukasza): https://crbug.com/650694: Add a "Get" flavour of the test once
 // the download works both for GET and POST requests.
-
-// Disabled on Windows. See http://crbug.com/653856.
-#if defined(OS_WIN)
-#define MAYBE_DownloadViaPost DISABLED_DownloadViaPost
-#else
-#define MAYBE_DownloadViaPost DownloadViaPost
-#endif
-
-IN_PROC_BROWSER_TEST_F(NavigatingExtensionPopupBrowserTest,
-                       MAYBE_DownloadViaPost) {
+IN_PROC_BROWSER_TEST_F(NavigatingExtensionPopupBrowserTest, DownloadViaPost) {
+  ASSERT_TRUE(CreateAndSetDownloadsDirectory(browser()));

[Devlin, 2016/10/07 21:11:38]

I don't feel too strongly, but if this is only used here, should we inline it?

[Łukasz Anforowicz, 2016/10/07 23:32:02]

Hmmm... not sure.

Arguments for inlining:
- More localized code
- Extra |downloads_directory_| field is not needed outside of the testcase.

Arguments for keeping as-is:
- Easy to code-search for CreateAndSetDownloadsDirectory
- The inlined setup code is not really core part of the test,
  so maybe better to keep it away from the main test logic

In the latest patchset I've decided to inline, because I really like having the
ScopedTempDir as a local variable (rather than a field of the test fixture). 
I've mentioned CreateAndSetDownloadsDirectory in a comment to hopefully still
enable searching the code for similar test code.

[Devlin, 2016/10/10 14:38:50]

Another option: create a method in the anonymous namespace that takes a
PrefService and returns a ScopedTempDir.  That would allow for separation of
logic while retaining isolation to this test and having the dir as a local
variable.

[Łukasz Anforowicz, 2016/10/10 15:08:57]

Good idea - thanks.  Done.

   content::DownloadTestObserverTerminal downloads_observer(
       content::BrowserContext::GetDownloadManager(browser()->profile()),
       1,  // == wait_count (only waiting for "download-test3.gif").