Re-commit: Add new PathService paths for Windows' All Users Desktop and Quick Launch folders.

base/path_service_unittest.cc

 // Copyright (c) 2012 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 "base/path_service.h"
 
 #include "base/basictypes.h"
 #include "base/file_util.h"
 #include "base/file_path.h"
 #include "base/scoped_temp_dir.h"
-#if defined(OS_WIN)
-#include "base/win/windows_version.h"
-#endif
+#include "base/string16.h"
+#include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/gtest/include/gtest/gtest-spi.h"
 #include "testing/platform_test.h"
 
+#if defined(OS_WIN)
+#include "base/win/windows_version.h"
+#endif
+
 namespace {
 
 // Returns true if PathService::Get returns true and sets the path parameter
 // to non-empty for the given PathService::DirType enumeration value.
 bool ReturnsValidPath(int dir_type) {
   FilePath path;
   bool result = PathService::Get(dir_type, &path);
 #if defined(OS_POSIX)
   // If chromium has never been started on this account, the cache path may not
   // exist.
   if (dir_type == base::DIR_CACHE)
-    return result && !path.value().empty();
+    return result && !path.empty();
 #endif
-  return result && !path.value().empty() && file_util::PathExists(path);
+#if defined(OS_LINUX)
+  // On the linux try-bots: a path is returned (e.g. /home/chrome-bot/Desktop),
+  // but it doesn't exist.
+  if (dir_type == base::DIR_USER_DESKTOP)
+    return (result && !path.empty() &&
+            path.value().find(L"Desktop") != string16::npos);

[grt (UTC plus 2), 2012/09/20 03:44:07]

IMO, this looks like a test failure waiting to happen.  Is it always called
"Desktop" on all machines all the time (not just the bot, but also every
developer's workstation)?

[gab, 2012/09/20 04:15:36]

Right, probably too dangerous, reverted.

+#elif defined(OS_WIN)

[grt (UTC plus 2), 2012/09/20 03:44:07]

the following is more consistent with lines 33 and 34:
#endif
#if defined(OS_WIN)

[gab, 2012/09/20 04:15:36]

I debated doing that, thanks for making my life/decision easy, done :)!

+  // On Windows XP, the Quick Launch folder for the "Default User" doesn't exist
+  // by default.
+  if (dir_type == base::DIR_DEFAULT_USER_QUICK_LAUNCH &&
+      base::win::GetVersion() <= base::win::VERSION_XP) {

[grt (UTC plus 2), 2012/09/20 03:44:07]

maybe this needs to be < base::win::VERSION_VISTA since VERSION_SERVER_2003 is
more like XP than Vista and covers XP Pro x64?

[gab, 2012/09/20 04:15:36]

Done.

+    return (result && !path.empty() &&
+            path.value().find(L"Default User") != string16::npos);

[gab, 2012/09/20 03:12:07]

Let me know what you think of adding the "find" to the extra validation here
(and on the OS_LINUX exception above).

My thinking is that since we can't do the full validation, we can at least do a
little bit more than assert !path.empty().

At the same time if we assert .find() we can probably remove the path.empty()
check... I only kept it as I don't like to rely on implementation and I'm
paranoically thinking that one day someone could decide that you must not call
path.value() if path.empty()... (i.e. that you could reset a FilePath without
reseting its internal state... but that's probably pushing it so feel free to
tell me I'm crazy!).

[grt (UTC plus 2), 2012/09/20 03:44:07]

Since pulling in Userenv.lib shouldn't be a problem for this test binary, what
do you think of calling GetDefaultUserProfileDirectory and confirm that it's a
prefix of |path|?  This avoids hardcoding "Default User".
chrome will explode if that ever becomes the case.

+  }
+#endif
+  return result && !path.empty() && file_util::PathExists(path);
 }
 
 #if defined(OS_WIN)
 // Function to test any directory keys that are not supported on some versions
 // of Windows. Checks that the function fails and that the returned path is
 // empty.
 bool ReturnsInvalidPath(int dir_type) {
   FilePath path;
   bool result = PathService::Get(dir_type, &path);
   return !result && path.empty();
 }
 #endif
 
 }  // namespace
 
 // On the Mac this winds up using some autoreleased objects, so we need to
 // be a PlatformTest.
 typedef PlatformTest PathServiceTest;
 
 // Test that all PathService::Get calls return a value and a true result
 // in the development environment.  (This test was created because a few
 // later changes to Get broke the semantics of the function and yielded the
 // correct value while returning false.)
 TEST_F(PathServiceTest, Get) {
-  for (int key = base::DIR_CURRENT; key < base::PATH_END; ++key) {
+  for (int key = base::PATH_START + 1; key < base::PATH_END; ++key) {
 #if defined(OS_ANDROID)
-    if (key == base::FILE_MODULE)
-      continue;  // Android doesn't implement FILE_MODULE;
+    if (key == base::FILE_MODULE || key == base::DIR_USER_DESKTOP)
+      continue;  // Android doesn't implement FILE_MODULE and DIR_USER_DESKTOP;
 #endif
     EXPECT_PRED1(ReturnsValidPath, key);
   }
 #if defined(OS_WIN)
   for (int key = base::PATH_WIN_START + 1; key < base::PATH_WIN_END; ++key) {
     bool valid = true;
     switch(key) {
       case base::DIR_LOCAL_APP_DATA_LOW:
         // DIR_LOCAL_APP_DATA_LOW is not supported prior Vista and is expected
         // to fail.
         valid = base::win::GetVersion() >= base::win::VERSION_VISTA;
         break;
       case base::DIR_APP_SHORTCUTS:
         // DIR_APP_SHORTCUTS is not supported prior Windows 8 and is expected to
         // fail.
         valid = base::win::GetVersion() >= base::win::VERSION_WIN8;
         break;
     }
 
     if (valid)
       EXPECT_TRUE(ReturnsValidPath(key)) << key;
     else
       EXPECT_TRUE(ReturnsInvalidPath(key)) << key;
   }
 #elif defined(OS_MACOSX)
   for (int key = base::PATH_MAC_START + 1; key < base::PATH_MAC_END; ++key) {
-      EXPECT_PRED1(ReturnsValidPath, key);
+    EXPECT_PRED1(ReturnsValidPath, key);
+  }
+#elif defined(OS_ANDROID)
+  for (int key = base::PATH_ANDROID_START + 1; key < base::PATH_ANDROID_END;
+       ++key) {
+    EXPECT_PRED1(ReturnsValidPath, key);
+  }
+#elif defined(OS_POSIX)
+  for (int key = base::PATH_POSIX_START + 1; key < base::PATH_POSIX_END;
+       ++key) {
+    EXPECT_PRED1(ReturnsValidPath, key);
   }
 #endif
 }
 
 // test that all versions of the Override function of PathService do what they
 // are supposed to do.
 TEST_F(PathServiceTest, Override) {
   int my_special_key = 666;
   ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   FilePath fake_cache_dir(temp_dir.path().AppendASCII("cache"));
   // PathService::Override should always create the path provided if it doesn't
   // exist.
   EXPECT_TRUE(PathService::Override(my_special_key, fake_cache_dir));
   EXPECT_TRUE(file_util::PathExists(fake_cache_dir));
 
   FilePath fake_cache_dir2(temp_dir.path().AppendASCII("cache2"));
   // PathService::OverrideAndCreateIfNeeded should obey the |create| parameter.
   PathService::OverrideAndCreateIfNeeded(my_special_key,
                                          fake_cache_dir2,
                                          false);
   EXPECT_FALSE(file_util::PathExists(fake_cache_dir2));
   EXPECT_TRUE(PathService::OverrideAndCreateIfNeeded(my_special_key,
                                                      fake_cache_dir2,
                                                      true));
   EXPECT_TRUE(file_util::PathExists(fake_cache_dir2));
 }