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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 6731033: Remove the path from PlatformFileInfo; it's cleaner just to pass it along as a (Closed)

Created:
9 years, 9 months ago by ericu
Modified:
9 years, 7 months ago
Reviewers:
kinuko, jam
CC:
chromium-reviews, Paweł Hajdan Jr., jam, pam+watch_chromium.org, brettw-cc_chromium.org, darin-cc_chromium.org, kinuko+watch
Visibility:
Public.

Description

Remove the path from PlatformFileInfo; it's cleaner just to pass it along as a separate parameter, and not clutter up a base type. BUG:none TEST:none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=79624

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 7

Patch Set 3 : '' #

Patch Set 4 : '' #

Patch Set 5 : '' #

Patch Set 6 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+84 lines, -49 lines) Patch
M base/platform_file.h View 1 2 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/extensions/extension_file_browser_private_api.cc View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/file_system/file_system_dispatcher_host.cc View 1 2 1 chunk +4 lines, -2 lines 0 comments Download
M content/common/common_param_traits.cc View 1 2 3 chunks +1 line, -5 lines 0 comments Download
M content/common/file_system/file_system_dispatcher.h View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M content/common/file_system/file_system_dispatcher.cc View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M content/common/file_system/webfilesystem_callback_dispatcher.h View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M content/common/file_system/webfilesystem_callback_dispatcher.cc View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M content/common/file_system/webfilewriter_impl.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M content/common/file_system_messages.h View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M webkit/fileapi/file_system_callback_dispatcher.h View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M webkit/fileapi/file_system_file_util.h View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M webkit/fileapi/file_system_file_util.cc View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M webkit/fileapi/file_system_file_util_proxy.h View 1 2 2 chunks +6 lines, -1 line 0 comments Download
M webkit/fileapi/file_system_file_util_proxy.cc View 1 2 2 chunks +3 lines, -2 lines 0 comments Download
M webkit/fileapi/file_system_operation.h View 1 2 1 chunk +6 lines, -3 lines 0 comments Download
M webkit/fileapi/file_system_operation.cc View 1 2 3 chunks +7 lines, -4 lines 0 comments Download
M webkit/fileapi/file_system_operation_unittest.cc View 1 2 6 chunks +12 lines, -4 lines 0 comments Download
M webkit/fileapi/local_file_system_file_util.h View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M webkit/fileapi/local_file_system_file_util.cc View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M webkit/plugins/ppapi/file_callbacks.h View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M webkit/plugins/ppapi/file_callbacks.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M webkit/tools/test_shell/simple_file_system.cc View 1 2 2 chunks +3 lines, -2 lines 0 comments Download
M webkit/tools/test_shell/simple_file_writer.cc View 1 2 2 chunks +6 lines, -3 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
ericu
Kinuko, here's the first cleanup I promised. John, you're on the list as you're an ...
9 years, 9 months ago (2011-03-25 01:15:06 UTC) #1
jam
content changes lgtm
9 years, 9 months ago (2011-03-25 17:58:07 UTC) #2
kinuko
LGTM modulo some nits. http://codereview.chromium.org/6731033/diff/2005/webkit/fileapi/file_system_file_util_proxy.h File webkit/fileapi/file_system_file_util_proxy.h (right): http://codereview.chromium.org/6731033/diff/2005/webkit/fileapi/file_system_file_util_proxy.h#newcode41 webkit/fileapi/file_system_file_util_proxy.h:41: const FilePath& wanna add comment? ...
9 years, 9 months ago (2011-03-25 22:39:53 UTC) #3
kinuko
http://codereview.chromium.org/6731033/diff/2005/webkit/fileapi/file_system_operation.h File webkit/fileapi/file_system_operation.h (right): http://codereview.chromium.org/6731033/diff/2005/webkit/fileapi/file_system_operation.h#newcode116 webkit/fileapi/file_system_operation.h:116: const FilePath&); On 2011/03/25 22:39:54, kinuko wrote: > nit: ...
9 years, 9 months ago (2011-03-25 22:44:43 UTC) #4
ericu
On Mar 25, 2011 6:44 PM, <kinuko@chromium.org> wrote: > > > http://codereview.chromium.org/6731033/diff/2005/webkit/fileapi/file_system_operation.h > File webkit/fileapi/file_system_operation.h ...
9 years, 9 months ago (2011-03-26 01:45:33 UTC) #5
kinuko
On 2011/03/26 01:45:33, ericu wrote: > On Mar 25, 2011 6:44 PM, <mailto:kinuko@chromium.org> wrote: > ...
9 years, 9 months ago (2011-03-26 03:25:18 UTC) #6
ericu
9 years, 9 months ago (2011-03-28 20:24:31 UTC) #7 
All fixed, thanks.
Will upload+submit as soon as I can validate this build [I synced a broken tree
by accident].

http://codereview.chromium.org/6731033/diff/2005/webkit/fileapi/file_system_f...
File webkit/fileapi/file_system_file_util_proxy.h (right):

http://codereview.chromium.org/6731033/diff/2005/webkit/fileapi/file_system_f...
webkit/fileapi/file_system_file_util_proxy.h:41: const FilePath&
On 2011/03/25 22:39:54, kinuko wrote:
> wanna add comment?
> /* platform_path */

Done.

http://codereview.chromium.org/6731033/diff/2005/webkit/fileapi/file_system_o...
File webkit/fileapi/file_system_operation.h (right):

http://codereview.chromium.org/6731033/diff/2005/webkit/fileapi/file_system_o...
webkit/fileapi/file_system_operation.h:116: const FilePath&);
On 2011/03/25 22:44:43, kinuko wrote:
> On 2011/03/25 22:39:54, kinuko wrote:
> > nit: in chromium I don't think we usually omit (unused) parameter names.
> 
> Found that this wasn't really true, though I remember I was told that by
someone
> before... I'll leave this tiny style nit to you.

I added them back in as "unused".

http://codereview.chromium.org/6731033/diff/2005/webkit/plugins/ppapi/file_ca...
File webkit/plugins/ppapi/file_callbacks.h (right):

http://codereview.chromium.org/6731033/diff/2005/webkit/plugins/ppapi/file_ca...
webkit/plugins/ppapi/file_callbacks.h:44: const FilePath&);
On 2011/03/25 22:39:54, kinuko wrote:
> ditto.

Done.

Powered by Google App Engine
This is Rietveld 408576698