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

Issue 11414222: Simplify local path resolution in fileBrowserPrivate API implementation (part 1). (Closed)

Created:
8 years ago by kinaba
Modified:
8 years ago
Reviewers:
dgozman
CC:
chromium-reviews, nkostylev+watch_chromium.org, Aaron Boodman, rginda+watch_chromium.org, oshima+watch_chromium.org, chromium-apps-reviews_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Visibility:
Public.

Description

Simplify local path resolution in fileBrowserPrivate API implementation (part 1). FileBrowserFunction::GetLocalPathsOnFileThreadAndRunCallbackOnUIThread() is used for two purposes. One is to parse file system URL (|file_path| field), and another is to resolve to the cache file path of Google Drive files (|local_path| field). The patch aims to separate the former use cases from the latter, before I'll add one more tweak here for selected path names in "Save as" dialog (see issue 140425). BUG=163075 TEST=Verify view/zip/copy-from-drive/copy-to-drive/format/mount-zip function works on Files.app. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=170443

Patch Set 1 #

Patch Set 2 : Resolve test failure and rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+108 lines, -293 lines) Patch
M chrome/browser/chromeos/extensions/file_browser_private_api.h View 10 chunks +1 line, -38 lines 0 comments Download
M chrome/browser/chromeos/extensions/file_browser_private_api.cc View 1 19 chunks +107 lines, -255 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
kinaba
+dgozman, could you review this CL?
8 years ago (2012-11-29 05:19:23 UTC) #1
kinaba
Oops, http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds/62710/steps/browser_tests/logs/FileBrowserMount This looks to be a real failure. Looking...
8 years ago (2012-11-29 08:53:39 UTC) #2
kinaba
Fixed the test failure, now it should be ready for reviewing. Can you have a ...
8 years ago (2012-11-30 03:53:52 UTC) #3
dgozman
LGTM Very nice refactoring!
8 years ago (2012-11-30 09:46:35 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kinaba@chromium.org/11414222/1003
8 years ago (2012-11-30 10:17:51 UTC) #5
commit-bot: I haz the power
Retried try job too often on linux_rel for step(s) content_unittests
8 years ago (2012-11-30 11:55:34 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kinaba@chromium.org/11414222/1003
8 years ago (2012-11-30 12:48:47 UTC) #7
commit-bot: I haz the power
8 years ago (2012-11-30 12:58:23 UTC) #8
Message was sent while issue was closed.
Change committed as 170443

Powered by Google App Engine
This is Rietveld 408576698