Untangle MediaScanner error handling.

Bug: 5056917

Change-Id: I1a7a73579e3ba4e9709459329fc1901a28b0f4b1

4 files changed, 178 insertions, 148 deletions

diff --git a/media/jni/android_media_MediaScanner.cpp b/media/jni/android_media_MediaScanner.cpp
index d0d2d1e..b88296f 100644
--- a/media/jni/android_media_MediaScanner.cpp
+++ b/media/jni/android_media_MediaScanner.cpp
@@ -46,6 +46,16 @@ struct fields_t {
 };
 static fields_t fields;
 
+static status_t checkAndClearExceptionFromCallback(JNIEnv* env, const char* methodName) {
+    if (env->ExceptionCheck()) {
+        LOGE("An exception was thrown by callback '%s'.", methodName);
+        LOGE_EX(env);
+        env->ExceptionClear();
+        return UNKNOWN_ERROR;
+    }
+    return OK;
+}
+
 class MyMediaScannerClient : public MediaScannerClient
 {
 public:
@@ -86,9 +96,7 @@ public:
         mEnv->DeleteGlobalRef(mClient);
     }
 
-    // Returns true if it succeeded, false if an exception occured
-    // in the Java code
-    virtual bool scanFile(const char* path, long long lastModified,
+    virtual status_t scanFile(const char* path, long long lastModified,
             long long fileSize, bool isDirectory, bool noMedia)
     {
         LOGV("scanFile: path(%s), time(%lld), size(%lld) and isDir(%d)",
@@ -96,27 +104,29 @@ public:
 
         jstring pathStr;
         if ((pathStr = mEnv->NewStringUTF(path)) == NULL) {
-            return false;
+            mEnv->ExceptionClear();
+            return NO_MEMORY;
         }
 
         mEnv->CallVoidMethod(mClient, mScanFileMethodID, pathStr, lastModified,
                 fileSize, isDirectory, noMedia);
 
         mEnv->DeleteLocalRef(pathStr);
-        return (!mEnv->ExceptionCheck());
+        return checkAndClearExceptionFromCallback(mEnv, "scanFile");
     }
 
-    // Returns true if it succeeded, false if an exception occured
-    // in the Java code
-    virtual bool handleStringTag(const char* name, const char* value)
+    virtual status_t handleStringTag(const char* name, const char* value)
     {
         LOGV("handleStringTag: name(%s) and value(%s)", name, value);
         jstring nameStr, valueStr;
         if ((nameStr = mEnv->NewStringUTF(name)) == NULL) {
-            return false;
+            mEnv->ExceptionClear();
+            return NO_MEMORY;
         }
         if ((valueStr = mEnv->NewStringUTF(value)) == NULL) {
-            return false;
+            mEnv->DeleteLocalRef(nameStr);
+            mEnv->ExceptionClear();
+            return NO_MEMORY;
         }
 
         mEnv->CallVoidMethod(
@@ -124,23 +134,22 @@ public:
 
         mEnv->DeleteLocalRef(nameStr);
         mEnv->DeleteLocalRef(valueStr);
-        return (!mEnv->ExceptionCheck());
+        return checkAndClearExceptionFromCallback(mEnv, "handleStringTag");
     }
 
-    // Returns true if it succeeded, false if an exception occured
-    // in the Java code
-    virtual bool setMimeType(const char* mimeType)
+    virtual status_t setMimeType(const char* mimeType)
     {
         LOGV("setMimeType: %s", mimeType);
         jstring mimeTypeStr;
         if ((mimeTypeStr = mEnv->NewStringUTF(mimeType)) == NULL) {
-            return false;
+            mEnv->ExceptionClear();
+            return NO_MEMORY;
         }
 
         mEnv->CallVoidMethod(mClient, mSetMimeTypeMethodID, mimeTypeStr);
 
         mEnv->DeleteLocalRef(mimeTypeStr);
-        return (!mEnv->ExceptionCheck());
+        return checkAndClearExceptionFromCallback(mEnv, "setMimeType");
     }
 
 private:
@@ -152,12 +161,6 @@ private:
 };
 
 
-static bool ExceptionCheck(void* env)
-{
-    LOGV("ExceptionCheck");
-    return ((JNIEnv *)env)->ExceptionCheck();
-}
-
 static MediaScanner *getNativeScanner_l(JNIEnv* env, jobject thiz)
 {
     return (MediaScanner *) env->GetIntField(thiz, fields.context);
@@ -190,7 +193,10 @@ android_media_MediaScanner_processDirectory(
     }
 
     MyMediaScannerClient myClient(env, client);
-    mp->processDirectory(pathStr, myClient, ExceptionCheck, env);
+    MediaScanResult result = mp->processDirectory(pathStr, myClient);
+    if (result == MEDIA_SCAN_RESULT_ERROR) {
+        LOGE("An error occurred while scanning directory '%s'.", pathStr);
+    }
     env->ReleaseStringUTFChars(path, pathStr);
 }
 
@@ -227,7 +233,10 @@ android_media_MediaScanner_processFile(
     }
 
     MyMediaScannerClient myClient(env, client);
-    mp->processFile(pathStr, mimeTypeStr, myClient);
+    MediaScanResult result = mp->processFile(pathStr, mimeTypeStr, myClient);
+    if (result == MEDIA_SCAN_RESULT_ERROR) {
+        LOGE("An error occurred while scanning file '%s'.", pathStr);
+    }
     env->ReleaseStringUTFChars(path, pathStr);
     if (mimeType) {
         env->ReleaseStringUTFChars(mimeType, mimeTypeStr);
diff --git a/media/libmedia/MediaScanner.cpp b/media/libmedia/MediaScanner.cpp
index 45bdff4..41f8593 100644
--- a/media/libmedia/MediaScanner.cpp
+++ b/media/libmedia/MediaScanner.cpp
@@ -47,16 +47,15 @@ const char *MediaScanner::locale() const {
     return mLocale;
 }
 
-status_t MediaScanner::processDirectory(
-        const char *path, MediaScannerClient &client,
-        ExceptionCheck exceptionCheck, void *exceptionEnv) {
+MediaScanResult MediaScanner::processDirectory(
+        const char *path, MediaScannerClient &client) {
     int pathLength = strlen(path);
     if (pathLength >= PATH_MAX) {
-        return UNKNOWN_ERROR;
+        return MEDIA_SCAN_RESULT_SKIPPED;
     }
     char* pathBuffer = (char *)malloc(PATH_MAX + 1);
     if (!pathBuffer) {
-        return UNKNOWN_ERROR;
+        return MEDIA_SCAN_RESULT_ERROR;
     }
 
     int pathRemaining = PATH_MAX - pathLength;
@@ -69,21 +68,18 @@ status_t MediaScanner::processDirectory(
 
     client.setLocale(locale());
 
-    status_t result =
-        doProcessDirectory(pathBuffer, pathRemaining, client, false, exceptionCheck, exceptionEnv);
+    MediaScanResult result = doProcessDirectory(pathBuffer, pathRemaining, client, false);
 
     free(pathBuffer);
 
     return result;
 }
 
-status_t MediaScanner::doProcessDirectory(
-        char *path, int pathRemaining, MediaScannerClient &client,
-        bool noMedia, ExceptionCheck exceptionCheck, void *exceptionEnv) {
+MediaScanResult MediaScanner::doProcessDirectory(
+        char *path, int pathRemaining, MediaScannerClient &client, bool noMedia) {
     // place to copy file or directory name
     char* fileSpot = path + strlen(path);
     struct dirent* entry;
-    struct stat statbuf;
 
     // Treat all files as non-media in directories that contain a  ".nomedia" file
     if (pathRemaining >= 8 /* strlen(".nomedia") */ ) {
@@ -99,76 +95,88 @@ status_t MediaScanner::doProcessDirectory(
 
     DIR* dir = opendir(path);
     if (!dir) {
-        LOGD("opendir %s failed, errno: %d", path, errno);
-        return UNKNOWN_ERROR;
+        LOGW("Error opening directory '%s', skipping: %s.", path, strerror(errno));
+        return MEDIA_SCAN_RESULT_SKIPPED;
     }
 
+    MediaScanResult result = MEDIA_SCAN_RESULT_OK;
     while ((entry = readdir(dir))) {
-        const char* name = entry->d_name;
-
-        // ignore "." and ".."
-        if (name[0] == '.' && (name[1] == 0 || (name[1] == '.' && name[2] == 0))) {
-            continue;
+        if (doProcessDirectoryEntry(path, pathRemaining, client, noMedia, entry, fileSpot)
+                == MEDIA_SCAN_RESULT_ERROR) {
+            result = MEDIA_SCAN_RESULT_ERROR;
+            break;
         }
+    }
+    closedir(dir);
+    return result;
+}
 
-        int nameLength = strlen(name);
-        if (nameLength + 1 > pathRemaining) {
-            // path too long!
-            continue;
-        }
-        strcpy(fileSpot, name);
-
-        int type = entry->d_type;
-        if (type == DT_UNKNOWN) {
-            // If the type is unknown, stat() the file instead.
-            // This is sometimes necessary when accessing NFS mounted filesystems, but
-            // could be needed in other cases well.
-            if (stat(path, &statbuf) == 0) {
-                if (S_ISREG(statbuf.st_mode)) {
-                    type = DT_REG;
-                } else if (S_ISDIR(statbuf.st_mode)) {
-                    type = DT_DIR;
-                }
-            } else {
-                LOGD("stat() failed for %s: %s", path, strerror(errno) );
+MediaScanResult MediaScanner::doProcessDirectoryEntry(
+        char *path, int pathRemaining, MediaScannerClient &client, bool noMedia,
+        struct dirent* entry, char* fileSpot) {
+    struct stat statbuf;
+    const char* name = entry->d_name;
+
+    // ignore "." and ".."
+    if (name[0] == '.' && (name[1] == 0 || (name[1] == '.' && name[2] == 0))) {
+        return MEDIA_SCAN_RESULT_SKIPPED;
+    }
+
+    int nameLength = strlen(name);
+    if (nameLength + 1 > pathRemaining) {
+        // path too long!
+        return MEDIA_SCAN_RESULT_SKIPPED;
+    }
+    strcpy(fileSpot, name);
+
+    int type = entry->d_type;
+    if (type == DT_UNKNOWN) {
+        // If the type is unknown, stat() the file instead.
+        // This is sometimes necessary when accessing NFS mounted filesystems, but
+        // could be needed in other cases well.
+        if (stat(path, &statbuf) == 0) {
+            if (S_ISREG(statbuf.st_mode)) {
+                type = DT_REG;
+            } else if (S_ISDIR(statbuf.st_mode)) {
+                type = DT_DIR;
             }
+        } else {
+            LOGD("stat() failed for %s: %s", path, strerror(errno) );
         }
-        if (type == DT_REG || type == DT_DIR) {
-            if (type == DT_DIR) {
-                bool childNoMedia = noMedia;
-                // set noMedia flag on directories with a name that starts with '.'
-                // for example, the Mac ".Trashes" directory
-                if (name[0] == '.')
-                    childNoMedia = true;
-
-                // report the directory to the client
-                if (stat(path, &statbuf) == 0) {
-                    client.scanFile(path, statbuf.st_mtime, 0, true, childNoMedia);
-                }
-
-                // and now process its contents
-                strcat(fileSpot, "/");
-                int err = doProcessDirectory(path, pathRemaining - nameLength - 1, client,
-                        childNoMedia, exceptionCheck, exceptionEnv);
-                if (err) {
-                    // pass exceptions up - ignore other errors
-                    if (exceptionCheck && exceptionCheck(exceptionEnv)) goto failure;
-                    LOGE("Error processing '%s' - skipping\n", path);
-                    continue;
-                }
-            } else {
-                stat(path, &statbuf);
-                client.scanFile(path, statbuf.st_mtime, statbuf.st_size, false, noMedia);
-                if (exceptionCheck && exceptionCheck(exceptionEnv)) goto failure;
+    }
+    if (type == DT_DIR) {
+        bool childNoMedia = noMedia;
+        // set noMedia flag on directories with a name that starts with '.'
+        // for example, the Mac ".Trashes" directory
+        if (name[0] == '.')
+            childNoMedia = true;
+
+        // report the directory to the client
+        if (stat(path, &statbuf) == 0) {
+            status_t status = client.scanFile(path, statbuf.st_mtime, 0,
+                    true /*isDirectory*/, childNoMedia);
+            if (status) {
+                return MEDIA_SCAN_RESULT_ERROR;
             }
         }
+
+        // and now process its contents
+        strcat(fileSpot, "/");
+        MediaScanResult result = doProcessDirectory(path, pathRemaining - nameLength - 1,
+                client, childNoMedia);
+        if (result == MEDIA_SCAN_RESULT_ERROR) {
+            return MEDIA_SCAN_RESULT_ERROR;
+        }
+    } else if (type == DT_REG) {
+        stat(path, &statbuf);
+        status_t status = client.scanFile(path, statbuf.st_mtime, statbuf.st_size,
+                false /*isDirectory*/, noMedia);
+        if (status) {
+            return MEDIA_SCAN_RESULT_ERROR;
+        }
     }
 
-    closedir(dir);
-    return OK;
-failure:
-    closedir(dir);
-    return -1;
+    return MEDIA_SCAN_RESULT_OK;
 }
 
 }  // namespace android
diff --git a/media/libmedia/MediaScannerClient.cpp b/media/libmedia/MediaScannerClient.cpp
index bd3596e..7a7aeb6 100644
--- a/media/libmedia/MediaScannerClient.cpp
+++ b/media/libmedia/MediaScannerClient.cpp
@@ -62,7 +62,7 @@ void MediaScannerClient::beginFile()
     mValues = new StringArray;
 }
 
-bool MediaScannerClient::addStringTag(const char* name, const char* value)
+status_t MediaScannerClient::addStringTag(const char* name, const char* value)
 {
     if (mLocaleEncoding != kEncodingNone) {
         // don't bother caching strings that are all ASCII.
@@ -212,8 +212,10 @@ void MediaScannerClient::endFile()
 
         // finally, push all name/value pairs to the client
         for (int i = 0; i < mNames->size(); i++) {
-            if (!handleStringTag(mNames->getEntry(i), mValues->getEntry(i)))
+            status_t status = handleStringTag(mNames->getEntry(i), mValues->getEntry(i));
+            if (status) {
                 break;
+            }
         }
     }
     // else addStringTag() has done all the work so we have nothing to do
diff --git a/media/libstagefright/StagefrightMediaScanner.cpp b/media/libstagefright/StagefrightMediaScanner.cpp
index 89faff7..571e8be 100644
--- a/media/libstagefright/StagefrightMediaScanner.cpp
+++ b/media/libstagefright/StagefrightMediaScanner.cpp
@@ -52,13 +52,13 @@ static bool FileHasAcceptableExtension(const char *extension) {
     return false;
 }
 
-static status_t HandleMIDI(
+static MediaScanResult HandleMIDI(
         const char *filename, MediaScannerClient *client) {
     // get the library configuration and do sanity check
     const S_EAS_LIB_CONFIG* pLibConfig = EAS_Config();
     if ((pLibConfig == NULL) || (LIB_VERSION != pLibConfig->libVersion)) {
         LOGE("EAS library/header mismatch\n");
-        return UNKNOWN_ERROR;
+        return MEDIA_SCAN_RESULT_ERROR;
     }
     EAS_I32 temp;
 
@@ -88,34 +88,41 @@ static status_t HandleMIDI(
     }
 
     if (result != EAS_SUCCESS) {
-        return UNKNOWN_ERROR;
+        return MEDIA_SCAN_RESULT_SKIPPED;
     }
 
     char buffer[20];
     sprintf(buffer, "%ld", temp);
-    if (!client->addStringTag("duration", buffer)) return UNKNOWN_ERROR;
-
-    return OK;
+    status_t status = client->addStringTag("duration", buffer);
+    if (status) {
+        return MEDIA_SCAN_RESULT_ERROR;
+    }
+    return MEDIA_SCAN_RESULT_OK;
 }
 
-status_t StagefrightMediaScanner::processFile(
+MediaScanResult StagefrightMediaScanner::processFile(
         const char *path, const char *mimeType,
         MediaScannerClient &client) {
     LOGV("processFile '%s'.", path);
 
     client.setLocale(locale());
     client.beginFile();
+    MediaScanResult result = processFileInternal(path, mimeType, client);
+    client.endFile();
+    return result;
+}
 
+MediaScanResult StagefrightMediaScanner::processFileInternal(
+        const char *path, const char *mimeType,
+        MediaScannerClient &client) {
     const char *extension = strrchr(path, '.');
 
     if (!extension) {
-        return UNKNOWN_ERROR;
+        return MEDIA_SCAN_RESULT_SKIPPED;
     }
 
     if (!FileHasAcceptableExtension(extension)) {
-        client.endFile();
-
-        return UNKNOWN_ERROR;
+        return MEDIA_SCAN_RESULT_SKIPPED;
     }
 
     if (!strcasecmp(extension, ".mid")
@@ -127,53 +134,57 @@ status_t StagefrightMediaScanner::processFile(
             || !strcasecmp(extension, ".rtx")
             || !strcasecmp(extension, ".ota")
             || !strcasecmp(extension, ".mxmf")) {
-        status_t status = HandleMIDI(path, &client);
-        if (status != OK) {
-            return status;
+        return HandleMIDI(path, &client);
+    }
+
+    sp<MediaMetadataRetriever> mRetriever(new MediaMetadataRetriever);
+
+    status_t status = mRetriever->setDataSource(path);
+    if (status) {
+        return MEDIA_SCAN_RESULT_ERROR;
+    }
+
+    const char *value;
+    if ((value = mRetriever->extractMetadata(
+                    METADATA_KEY_MIMETYPE)) != NULL) {
+        status = client.setMimeType(value);
+        if (status) {
+            return MEDIA_SCAN_RESULT_ERROR;
         }
-    } else {
-        sp<MediaMetadataRetriever> mRetriever(new MediaMetadataRetriever);
-
-         if (mRetriever->setDataSource(path) == OK) {
-            const char *value;
-            if ((value = mRetriever->extractMetadata(
-                            METADATA_KEY_MIMETYPE)) != NULL) {
-                client.setMimeType(value);
-            }
+    }
 
-            struct KeyMap {
-                const char *tag;
-                int key;
-            };
-            static const KeyMap kKeyMap[] = {
-                { "tracknumber", METADATA_KEY_CD_TRACK_NUMBER },
-                { "discnumber", METADATA_KEY_DISC_NUMBER },
-                { "album", METADATA_KEY_ALBUM },
-                { "artist", METADATA_KEY_ARTIST },
-                { "albumartist", METADATA_KEY_ALBUMARTIST },
-                { "composer", METADATA_KEY_COMPOSER },
-                { "genre", METADATA_KEY_GENRE },
-                { "title", METADATA_KEY_TITLE },
-                { "year", METADATA_KEY_YEAR },
-                { "duration", METADATA_KEY_DURATION },
-                { "writer", METADATA_KEY_WRITER },
-                { "compilation", METADATA_KEY_COMPILATION },
-                { "isdrm", METADATA_KEY_IS_DRM },
-            };
-            static const size_t kNumEntries = sizeof(kKeyMap) / sizeof(kKeyMap[0]);
-
-            for (size_t i = 0; i < kNumEntries; ++i) {
-                const char *value;
-                if ((value = mRetriever->extractMetadata(kKeyMap[i].key)) != NULL) {
-                    client.addStringTag(kKeyMap[i].tag, value);
-                }
+    struct KeyMap {
+        const char *tag;
+        int key;
+    };
+    static const KeyMap kKeyMap[] = {
+        { "tracknumber", METADATA_KEY_CD_TRACK_NUMBER },
+        { "discnumber", METADATA_KEY_DISC_NUMBER },
+        { "album", METADATA_KEY_ALBUM },
+        { "artist", METADATA_KEY_ARTIST },
+        { "albumartist", METADATA_KEY_ALBUMARTIST },
+        { "composer", METADATA_KEY_COMPOSER },
+        { "genre", METADATA_KEY_GENRE },
+        { "title", METADATA_KEY_TITLE },
+        { "year", METADATA_KEY_YEAR },
+        { "duration", METADATA_KEY_DURATION },
+        { "writer", METADATA_KEY_WRITER },
+        { "compilation", METADATA_KEY_COMPILATION },
+        { "isdrm", METADATA_KEY_IS_DRM },
+    };
+    static const size_t kNumEntries = sizeof(kKeyMap) / sizeof(kKeyMap[0]);
+
+    for (size_t i = 0; i < kNumEntries; ++i) {
+        const char *value;
+        if ((value = mRetriever->extractMetadata(kKeyMap[i].key)) != NULL) {
+            status = client.addStringTag(kKeyMap[i].tag, value);
+            if (status) {
+                return MEDIA_SCAN_RESULT_ERROR;
             }
         }
     }
 
-    client.endFile();
-
-    return OK;
+    return MEDIA_SCAN_RESULT_OK;
 }
 
 char *StagefrightMediaScanner::extractAlbumArt(int fd) {