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

Issue 9320059: Define FilePath::NormalizePathSeparators on all platforms (Closed)

Created:
8 years, 10 months ago by kinuko
Modified:
8 years, 10 months ago
Reviewers:
Mark Mentovai, tony
CC:
chromium-reviews, jam, mihaip+watch_chromium.org, joi+watch-content_chromium.org, Aaron Boodman, darin-cc_chromium.org, rdsmith+dwatch_chromium.org, brettw-cc_chromium.org, kinuko+watch
Visibility:
Public.

Description

Define FilePath::NormalizePathSeparators on all platforms I assume the method FilePath::NormalizeWindowsPathSeparators() is intentionally defined only on Windows, but recently I found myself trying to add a static method (named NormalizePathSeparators()) which calls NormalizeWindowsPathSeparators() or does nothing with platform ifdefs, and then found that there's another place defining the same static method. Maybe we could just add the common method in FilePath then? It'd at least make the code cleaner at several callsites. I don't think this has visible negative performance impact with optimization build. BUG=none TEST=FilePathTest.NormalizePathSeparators and all other existing tests Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=121380

Patch Set 1 : '' #

Patch Set 2 : build fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -76 lines) Patch
M base/file_path.h View 1 chunk +3 lines, -4 lines 0 comments Download
M base/file_path.cc View 1 chunk +4 lines, -2 lines 0 comments Download
M base/file_path_unittest.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/extension_protocols.cc View 2 chunks +2 lines, -6 lines 0 comments Download
M chrome/browser/extensions/extension_service_unittest.cc View 2 chunks +10 lines, -26 lines 0 comments Download
M chrome/browser/ui/views/aura/app_list/extension_app_item.cc View 1 chunk +1 line, -3 lines 0 comments Download
M chrome/browser/ui/webui/extensions/extension_icon_source.cc View 1 chunk +1 line, -3 lines 0 comments Download
M content/browser/download/download_manager_impl_unittest.cc View 1 1 chunk +1 line, -5 lines 0 comments Download
M webkit/fileapi/file_system_util.cc View 1 chunk +2 lines, -6 lines 0 comments Download
M webkit/fileapi/isolated_context.cc View 1 chunk +1 line, -5 lines 0 comments Download
M webkit/fileapi/isolated_context_unittest.cc View 1 3 chunks +5 lines, -11 lines 0 comments Download
M webkit/fileapi/obfuscated_file_util_unittest.cc View 1 chunk +1 line, -3 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
kinuko
Now that we have several callsites for NormalizeWindowsPathSeparators (all with ifdefs) I thought it might ...
8 years, 10 months ago (2012-02-09 16:35:22 UTC) #1
Mark Mentovai
8 years, 10 months ago (2012-02-09 16:43:04 UTC) #2
This should be fine. LGTM.

Powered by Google App Engine
This is Rietveld 408576698