MediaGalleriesCustomBindings: Ensure blobs aren't GC'd when getMetadata runs.

chrome/renderer/resources/extensions/media_galleries_custom_bindings.js

Index: chrome/renderer/resources/extensions/media_galleries_custom_bindings.js
diff --git a/chrome/renderer/resources/extensions/media_galleries_custom_bindings.js b/chrome/renderer/resources/extensions/media_galleries_custom_bindings.js
index f838086848c1b555f6a5e490288202d191f99a08..841cefa387f2300aad1ef2795b35d3a63213699f 100644
--- a/chrome/renderer/resources/extensions/media_galleries_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/media_galleries_custom_bindings.js
@@ -8,6 +8,7 @@ var binding = require('binding').Binding.create('mediaGalleries');
 var blobNatives = requireNative('blob_natives');
 var mediaGalleriesNatives = requireNative('mediaGalleries');
 
+var mediaFilesMetadata = {};

[tommycli, 2014/09/29 21:50:00]

This makes it sound like you're storing the metadata of the media file rather
than a reference to a Blob you need to hold onto because of a pending
getMetadata request.

Off the top of my head, 'blobsWithPendingMetadataRequests'? Ugh, I hope you can
think of a better name than that one...

[Oren Blasberg, 2014/09/29 22:05:48]

Done.

 var mediaGalleriesMetadata = {};
 
 function createFileSystemObjectsAndUpdateMetadata(response) {
@@ -101,6 +102,10 @@ binding.registerCustomHook(function(bindingsAPI, extensionId) {
   apiFunctions.setUpdateArgumentsPostValidate('getMetadata',
       function(mediaFile, options, callback) {
     var blobUuid = blobNatives.GetBlobUuid(mediaFile)
+    // Store the blob in a global object to keep its refcount nonzero -- this

[tommycli, 2014/09/29 21:50:00]

Perfect comment :)

[Oren Blasberg, 2014/09/29 22:05:48]

Thanks Tommy!

+    // prevents the object from being garbage collected before any metadata
+    // parsing gets to occur (see crbug.com/415792).
+    mediaFilesMetadata[blobUuid] = mediaFile;
     return [blobUuid, options, callback];
   });
 
@@ -118,6 +123,9 @@ binding.registerCustomHook(function(bindingsAPI, extensionId) {
     if (request.callback)
       request.callback(response.metadata);
     request.callback = null;
+
+    var uuid = request.args[0];

[tommycli, 2014/09/29 21:50:00]

Maybe a short one-line comment since this is not super obvious.

And don't worry, there's precedent for doing this:
https://code.google.com/p/chromium/codesearch/#search/&q=args%20custom_bindin...

[Oren Blasberg, 2014/09/29 22:05:48]

What sort of comment would be reasonable here? Remember, I'm still not sure how
args[0] ends up being the uuid after all.

[tommycli, 2014/09/29 22:09:25]

It's the uuid because of line 109: return [blobUuid, options, callback].

Maybe this doesn't even need a comment. I don't have a strong feeling about
this. You're the maintainer, you can make a judgement call.

+    delete mediaFilesMetadata[uuid];
   });
 });