[Downloads] Consolidate MOTW annotation APIs into a single API.

content/browser/download/quarantine_linux.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "content/browser/download/file_metadata_linux.h"
+#include "content/browser/download/quarantine.h"
 
 #include <stddef.h>
 #include <sys/types.h>
 #include <sys/xattr.h>
 
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
+#include "base/threading/thread_restrictions.h"
+#include "content/browser/download/quarantine_constants_linux.h"
 #include "url/gurl.h"
 
 namespace content {
 
-const char kSourceURLAttrName[] = "user.xdg.origin.url";
-const char kReferrerURLAttrName[] = "user.xdg.referrer.url";
+const char kSourceURLExtendedAttrName[] = "user.xdg.origin.url";
+const char kReferrerURLExtendedAttrName[] = "user.xdg.referrer.url";
 
-static void SetExtendedFileAttribute(const char* path, const char* name,
-                                     const char* value, size_t value_size,
-                                     int flags) {
+namespace {
+
+bool SetExtendedFileAttribute(const char* path,
+                              const char* name,
+                              const char* value,
+                              size_t value_size,
+                              int flags) {
+  base::ThreadRestrictions::AssertIOAllowed();
   int result = setxattr(path, name, value, value_size, flags);
   if (result) {
-    DPLOG(ERROR)
-        << "Could not set extended attribute " << name << " on file " << path;
+    DPLOG(ERROR) << "Could not set extended attribute " << name << " on file "
+                 << path;
+    return false;
   }
+  return true;
 }
 
-void AddOriginMetadataToFile(const base::FilePath& file, const GURL& source,
-                             const GURL& referrer) {
+}  // namespace
+
+QuarantineFileResult QuarantineFile(const base::FilePath& file,
+                                    const GURL& source_url,
+                                    const GURL& referrer_url,
+                                    const std::string& client_guid) {
   DCHECK(base::PathIsWritable(file));
-  if (source.is_valid()) {
-    SetExtendedFileAttribute(file.value().c_str(), kSourceURLAttrName,
-        source.spec().c_str(), source.spec().length(), 0);
+  bool succeeded = true;
+  if (source_url.is_valid()) {

[svaldez, 2016/07/11 14:55:45]

Should you be early failing if setting the SourceURL attribute fails?

[asanka, 2016/09/12 15:36:37]

Currently it's possible for the referrer URL to be invalid/empty, but the source
must always be valid. So I've modified the patch to:

* Attempt to set the source and referrer attributes if the corresponding URLs
are valid. No short circuits.
* The return value would be ANNOTATION_FAILED if any of the following is true:
  * Source URL was invalid.
  * Source URL was valid, but setting the source attribute failed.
  * Referrer URL was valid, but setting the referrer attribute failed.

+    succeeded = SetExtendedFileAttribute(
+        file.value().c_str(), kSourceURLExtendedAttrName,
+        source_url.spec().c_str(), source_url.spec().length(), 0);
   }
-  if (referrer.is_valid()) {
-    SetExtendedFileAttribute(file.value().c_str(), kReferrerURLAttrName,
-        referrer.spec().c_str(), referrer.spec().length(), 0);
+  if (referrer_url.is_valid()) {
+    succeeded = SetExtendedFileAttribute(file.value().c_str(),
+                                         kReferrerURLExtendedAttrName,
+                                         referrer_url.spec().c_str(),
+                                         referrer_url.spec().length(), 0) &&
+                succeeded;
   }
+  return succeeded ? QuarantineFileResult::OK
+                   : QuarantineFileResult::ANNOTATION_FAILED;
 }
 
 }  // namespace content