arc: Exclude non-media files from Android media scanning.

chrome/browser/chromeos/arc/arc_downloads_watcher_service.cc

Index: chrome/browser/chromeos/arc/arc_downloads_watcher_service.cc
diff --git a/chrome/browser/chromeos/arc/arc_downloads_watcher_service.cc b/chrome/browser/chromeos/arc/arc_downloads_watcher_service.cc
index b440e327f85a4fca1a82ee1a5bb0d38af45a31e4..1219b9985517308c78b32467d8fc3e677d5c3ffc 100644
--- a/chrome/browser/chromeos/arc/arc_downloads_watcher_service.cc
+++ b/chrome/browser/chromeos/arc/arc_downloads_watcher_service.cc
@@ -4,6 +4,8 @@
 
 #include "chrome/browser/chromeos/arc/arc_downloads_watcher_service.h"
 
+#include <algorithm>
+#include <iterator>
 #include <map>
 #include <memory>
 #include <utility>
@@ -13,6 +15,7 @@
 #include "base/files/file_path.h"
 #include "base/files/file_path_watcher.h"
 #include "base/memory/ptr_util.h"
+#include "base/strings/string_util.h"
 #include "base/time/time.h"
 #include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -38,6 +41,55 @@ const base::FilePath::CharType kAndroidDownloadDir[] =
 const base::TimeDelta kBuildTimestampMapDelay =
     base::TimeDelta::FromMilliseconds(1000);
 
+// The set of media file extensions supported by Android MediaScanner.
+// Entries must be sorted by lexicographical order for binary search.

[Yusuke Sato, 2016/09/30 22:51:23]

and must be lower-case, I think.

[Shuhei Takahashi, 2016/10/03 08:48:52]

Done.

+// This list should be in sync with
+// frameworks/base/media/java/android/media/MediaScanner.java.

[Yusuke Sato, 2016/09/30 22:51:23]

What about adding a git hash to document the revision you used?

[Shuhei Takahashi, 2016/10/03 08:48:52]

Mentioned that this list from aosp-marshmallow. Not a specific commit, but I
believe the list is fairly stable in aosp-marshmallow branch.

+const char* kAndroidSupportedMediaExtensions[] = {
+    ".3g2",    // FILE_TYPE_3GPP2, video/3gpp2
+    ".3gp",    // FILE_TYPE_3GPP, video/3gpp
+    ".3gpp",   // FILE_TYPE_3GPP, video/3gpp
+    ".3gpp2",  // FILE_TYPE_3GPP2, video/3gpp2
+    ".aac",    // FILE_TYPE_AAC, audio/aac, audio/aac-adts
+    ".amr",    // FILE_TYPE_AMR, audio/amr
+    ".asf",    // FILE_TYPE_ASF, video/x-ms-asf
+    ".avi",    // FILE_TYPE_AVI, video/avi
+    ".awb",    // FILE_TYPE_AWB, audio/amr-wb
+    ".bmp",    // FILE_TYPE_BMP, image/x-ms-bmp
+    ".fl",     // FILE_TYPE_FL, application/x-android-drm-fl
+    ".gif",    // FILE_TYPE_GIF, image/gif
+    ".imy",    // FILE_TYPE_IMY, audio/imelody
+    ".jpeg",   // FILE_TYPE_JPEG, image/jpeg
+    ".jpg",    // FILE_TYPE_JPEG, image/jpeg
+    ".m4a",    // FILE_TYPE_M4A, audio/mp4
+    ".m4v",    // FILE_TYPE_M4V, video/mp4
+    ".mid",    // FILE_TYPE_MID, audio/midi
+    ".midi",   // FILE_TYPE_MID, audio/midi
+    ".mka",    // FILE_TYPE_MKA, audio/x-matroska
+    ".mkv",    // FILE_TYPE_MKV, video/x-matroska
+    ".mp3",    // FILE_TYPE_MP3, audio/mpeg
+    ".mp4",    // FILE_TYPE_MP4, video/mp4
+    ".mpeg",   // FILE_TYPE_MP4, video/mpeg, video/mp2p
+    ".mpg",    // FILE_TYPE_MP4, video/mpeg, video/mp2p
+    ".mpga",   // FILE_TYPE_MP3, audio/mpeg
+    ".mxmf",   // FILE_TYPE_MID, audio/midi
+    ".oga",    // FILE_TYPE_OGG, application/ogg
+    ".ogg",    // FILE_TYPE_OGG, audio/ogg, application/ogg
+    ".ota",    // FILE_TYPE_MID, audio/midi
+    ".png",    // FILE_TYPE_PNG, image/png
+    ".rtttl",  // FILE_TYPE_MID, audio/midi
+    ".rtx",    // FILE_TYPE_MID, audio/midi
+    ".smf",    // FILE_TYPE_SMF, audio/sp-midi
+    ".ts",     // FILE_TYPE_MP2TS, video/mp2ts
+    ".wav",    // FILE_TYPE_WAV, audio/x-wav
+    ".wbmp",   // FILE_TYPE_WBMP, image/vnd.wap.wbmp
+    ".webm",   // FILE_TYPE_WEBM, video/webm
+    ".webp",   // FILE_TYPE_WEBP, image/webp
+    ".wma",    // FILE_TYPE_WMA, audio/x-ms-wma
+    ".wmv",    // FILE_TYPE_WMV, video/x-ms-wmv
+    ".xmf",    // FILE_TYPE_MID, audio/midi
+};
+
 // Compares two TimestampMaps and returns the list of file paths added/removed
 // or whose timestamp have changed.
 std::vector<base::FilePath> CollectChangedPaths(
@@ -75,6 +127,16 @@ std::vector<base::FilePath> CollectChangedPaths(
   return changed_paths;
 }
 
+// Returns true if the extension of the file path looks like a media file.
+bool HasMediaExtension(const base::FilePath& path) {

[Yusuke Sato, 2016/09/30 22:51:23]

Would you mind adding
chrome/browser/chromeos/arc/arc_downloads_watcher_service_unittest.cc? I know
this function is kind of trivial but I'd like to see some tests for the file...

[Shuhei Takahashi, 2016/10/03 08:48:52]

Yes that's good, and actually this function had a bug. My brain was exhausted at
the time I wrote this code...

+  const std::string extension = base::ToLowerASCII(path.Extension());

[Yusuke Sato, 2016/09/30 22:51:23]

..and add DCHECK(std::is_sorted(...)); check here?

[Shuhei Takahashi, 2016/10/03 08:48:52]

Done.

+  auto iter = std::lower_bound(
+      std::begin(kAndroidSupportedMediaExtensions),
+      std::end(kAndroidSupportedMediaExtensions), extension,
+      [](const char* a, const std::string& b) { return a == b; });

[Yusuke Sato, 2016/09/30 22:51:23]

nit: any reason to create a string every time?

 ...   extension.c_str(),
 [](const char* a, const char* b) { return !strcmp(a, b); });

maybe?

[Shuhei Takahashi, 2016/10/03 08:48:52]

Sounds good, done.

+  return iter != std::end(kAndroidSupportedMediaExtensions);
+}
+
 // Scans files under |downloads_dir| recursively and builds a map from file
 // paths (in Android filesystem) to last modified timestamps.
 TimestampMap BuildTimestampMap(base::FilePath downloads_dir) {
@@ -86,6 +148,9 @@ TimestampMap BuildTimestampMap(base::FilePath downloads_dir) {
                                   base::FileEnumerator::FILES);
   for (base::FilePath cros_path = enumerator.Next(); !cros_path.empty();
        cros_path = enumerator.Next()) {
+    // Skip non-media files for efficiency.
+    if (!HasMediaExtension(cros_path))
+      continue;
     // Android file path can be obtained by replacing |downloads_dir| prefix
     // with |kAndroidDownloadDir|.
     base::FilePath android_path(kAndroidDownloadDir);