Be explicit about target type in platform_util::OpenItem()

chrome/browser/platform_util.h

Index: chrome/browser/platform_util.h
diff --git a/chrome/browser/platform_util.h b/chrome/browser/platform_util.h
index 916f3a307c78cb99af4dae2f82191483985a8475..bc4fa4aac921d1f580cfe5f8c2b0fb43ff36a013 100644
--- a/chrome/browser/platform_util.h
+++ b/chrome/browser/platform_util.h
@@ -22,12 +22,18 @@ namespace platform_util {
 // Show the given file in a file manager. If possible, select the file.
 // The |profile| is used to determine the running profile of file manager app
 // in Chrome OS only. Not used in other platforms.
-// Must be called from the UI thread.
+// Must be called on the UI thread.
 void ShowItemInFolder(Profile* profile, const base::FilePath& full_path);
 
-// Open the given file in the desktop's default manner.
-// Must be called from the UI thread.
-void OpenItem(Profile* profile, const base::FilePath& full_path);
+// Opens the given file in the desktop's default manner. |full_path| must point
+// to an existing file. If |full_path| doesn't refer to an existing file, then

[James Hawkins, 2014/09/29 21:09:14]

This secondary condition "If ... doesn't refer to an existing file" is confusing
because the statement preceding it says it shouldn't be possible.  When the
documentation says 'must', we must assert that the condition is true.  Any other
behavior must be undocumented.

[asanka, 2014/09/30 23:07:46]

Yup. I updated the comments for ShowItemInFolder(), OpenFile(), and
OpenFolder(). I relaxed the requirement that |full_path| must exist and point to
files/folders since that's often not practical for the call site to determine
prior to calling these functions.

They are also currently fire-and-forget with no feedback of whether the
operation succeeded. This behavior reflects the nature of the platform level
APIs that are used to implement them. So I only documented the restrictions that
are enforced within Chrome before the request is passed down to the underlying
platform.

+// no action will be taken. Must be called on the UI thread.
+void OpenFile(Profile* profile, const base::FilePath& full_path);
+
+// Opens the given folder in the file manager. No action will be taken if

[James Hawkins, 2014/09/29 21:09:14]

Why do we not say anything about |full_path| pointing to an existing folder, as
we do for OpenFile?

[James Hawkins, 2014/09/29 21:09:14]

What is the 'file manager'?  Perhaps we should be less-specific, as is the case
for OpenFile where we refer to the desktop's default manner.

[asanka, 2014/09/30 23:07:46]

Comment updated.

[asanka, 2014/09/30 23:07:46]

Comment updated.

+// |full_path| does not refer to an existing folder. Must be called on the UI
+// thread.
+void OpenFolder(Profile* profile, const base::FilePath& full_path);
 
 // Open the given external protocol URL in the desktop's default manner.
 // (For example, mailto: URLs in the default mail user agent.)