Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(81)

Issue 10958009: Revert 157667 - Add new PathService paths for Windows' All Users Desktop and Quick Launch folders. (Closed)

Created:
8 years, 3 months ago by wjia(left Chromium)
Modified:
7 years, 8 months ago
Reviewers:
wjia1, gab
CC:
chromium-reviews, tfarina, erikwright+watch_chromium.org, rdsmith+dwatch_chromium.org, grt+watch_chromium.org
Visibility:
Public.

Description

Revert 157667 - Add new PathService paths for Windows' All Users Desktop and Quick Launch folders. The previous patch failed PathServiceTest.Get. This allows usage of PathService to cache the paths and more importantly to mock them in shortcut tests! Also move chrome::DIR_USER_DESKTOP to base::DIR_USER_DESKTOP; this is really where it belongs. In fact it is only in chrome_paths.h because it used to be called DIR_DEFAULT_DOWNLOAD and cpu@ renamed it to DIR_USER_DESKTOP in http://crrev.com/1753 (early days!) after that it started to be used all over the place as the Desktop path. Finally bringing it to base_paths.h, beside DIR_START_MENU and friends, is the right thing to do imo. BUG=148539 TEST=Quick Launch shortcut installed in the right place on XP (both Default and current user) Desktop shortcuts installed in the right place (both All Users and per-user installs). installer_util_unittests.exe --gtest_filter=ShellUtilShortcutTest* unit_tests.exe --gtest_filter=ProfileShortcutManagerTest* base_unittests --gtest_filter=PathServiceTest* Review URL: https://chromiumcodereview.appspot.com/10910209 TBR=gab@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=157680

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+152 lines, -215 lines) Patch
M base/base.gypi View 1 chunk +0 lines, -1 line 0 comments Download
M base/base_paths.h View 2 chunks +11 lines, -5 lines 0 comments Download
M base/base_paths.cc View 1 chunk +1 line, -1 line 0 comments Download
M base/base_paths_android.h View 1 chunk +0 lines, -1 line 0 comments Download
M base/base_paths_android.cc View 2 chunks +17 lines, -15 lines 0 comments Download
MM base/base_paths_mac.mm View 5 chunks +7 lines, -10 lines 0 comments Download
D base/base_paths_posix.h View 1 chunk +0 lines, -29 lines 0 comments Download
M base/base_paths_posix.cc View 4 chunks +4 lines, -9 lines 0 comments Download
M base/base_paths_win.h View 1 chunk +0 lines, -6 lines 0 comments Download
M base/base_paths_win.cc View 4 chunks +3 lines, -57 lines 0 comments Download
M base/path_service.cc View 3 chunks +4 lines, -6 lines 0 comments Download
M base/path_service_unittest.cc View 4 chunks +9 lines, -27 lines 0 comments Download
M chrome/browser/download/download_util.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc View 3 chunks +2 lines, -8 lines 0 comments Download
M chrome/browser/shell_integration_linux.cc View 4 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/shell_integration_win.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/views/test/ui_test_utils_win.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/web_applications/web_app_ui.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/web_applications/web_app_win.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/common/chrome_paths.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/chrome_paths.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/common/chrome_paths_android.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/common/chrome_paths_internal.h View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/common/chrome_paths_linux.cc View 2 chunks +6 lines, -2 lines 0 comments Download
M chrome/common/chrome_paths_mac.mm View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/common/chrome_paths_win.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/installer/util/shell_util.cc View 1 chunk +34 lines, -6 lines 0 comments Download
M chrome/installer/util/shell_util_unittest.cc View 7 chunks +20 lines, -24 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
wjia(left Chromium)
8 years, 3 months ago (2012-09-20 01:59:13 UTC) #1
wjia1
FYI, PathServiceTest.Get failed in a row: http://build.chromium.org/p/chromium.win/buildstatus?builder=XP%20Tests%20%28dbg%29%281%29&number=27091 http://build.chromium.org/p/chromium.win/buildstatus?builder=XP%20Tests%20%28dbg%29%281%29&number=27092
8 years, 3 months ago (2012-09-20 02:03:25 UTC) #2
gab
lgtm, thanks.
8 years, 3 months ago (2012-09-20 02:20:25 UTC) #3
Finnur
Hmmm... here's something a bit odd. I'm investigating this bug: https://code.google.com/p/chromium/issues/detail?id=230766 I bisected the builds ...
7 years, 8 months ago (2013-04-15 14:01:21 UTC) #4
Finnur
Hmmm... it looks like this was checked in again later. But this time the bug ...
7 years, 8 months ago (2013-04-15 14:22:48 UTC) #5
Finnur
7 years, 8 months ago (2013-04-15 15:39:36 UTC) #6
Message was sent while issue was closed.
Looks like this was a false alarm. Threaded compositing is the suspected culprit
in this case. Move along, nothing to see. :)

Powered by Google App Engine
This is Rietveld 408576698