Skip to content

Commit

Permalink
Change the Media Galleries file system name back to a generic string.
Browse files Browse the repository at this point in the history 
Now that we have getMediaFileSystemMetadata, we can remove the JSON string we had stuffed into the name field.

BUG=170138

Review URL: https://chromiumcodereview.appspot.com/11946025

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@192238 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
vandebo@chromium.org committed Apr 4, 2013
1 parent 7e8ce78 commit fca4da9
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 118 deletions.
19 changes: 7 additions & 12 deletions chrome/browser/extensions/api/media_galleries/media_galleries_api.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,11 @@ const char kDisallowedByPolicy[] =
"Media Galleries API is disallowed by policy: ";
const char kInvalidInteractive[] = "Unknown value for interactive.";

const char kIsRemovableKey[] = "isRemovable";
const char kDeviceIdKey[] = "deviceId";
const char kGalleryIdKey[] = "galleryId";
const char kIsMediaDeviceKey[] = "isMediaDevice";
const char kIsRemovableKey[] = "isRemovable";
const char kNameKey[] = "name";

// Checks whether the MediaGalleries API is currently accessible (it may be
// disallowed even if an extension has the requisite permission).
Expand Down Expand Up @@ -146,7 +149,6 @@ void MediaGalleriesGetMediaFileSystemsFunction::ReturnGalleries(
APIPermission::kMediaGalleries, &read_param);

const int child_id = rvh->GetProcess()->GetID();
std::set<std::string> file_system_names;
base::ListValue* list = new base::ListValue();
for (size_t i = 0; i < filesystems.size(); i++) {
scoped_ptr<base::DictionaryValue> file_system_dict_value(
Expand All @@ -157,21 +159,14 @@ void MediaGalleriesGetMediaFileSystemsFunction::ReturnGalleries(
file_system_dict_value->SetStringWithoutPathExpansion(
"fsid", filesystems[i].fsid);

// The name must be unique according to the HTML5 File System API spec.
if (ContainsKey(file_system_names, filesystems[i].name)) {
NOTREACHED() << "Duplicate file system name: " << filesystems[i].name;
continue;
}
file_system_names.insert(filesystems[i].name);
file_system_dict_value->SetStringWithoutPathExpansion(
MediaFileSystemRegistry::kNameKey, filesystems[i].name);
kNameKey, filesystems[i].name);
file_system_dict_value->SetStringWithoutPathExpansion(
MediaFileSystemRegistry::kGalleryIdKey,
kGalleryIdKey,
base::Uint64ToString(filesystems[i].pref_id));
if (!filesystems[i].transient_device_id.empty()) {
file_system_dict_value->SetStringWithoutPathExpansion(
MediaFileSystemRegistry::kDeviceIdKey,
filesystems[i].transient_device_id);
kDeviceIdKey, filesystems[i].transient_device_id);
}
file_system_dict_value->SetBooleanWithoutPathExpansion(
kIsRemovableKey, filesystems[i].removable);
Expand Down
49 changes: 2 additions & 47 deletions chrome/browser/media_galleries/media_file_system_registry.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "base/bind.h"
#include "base/callback.h"
#include "base/files/file_path.h"
#include "base/json/json_writer.h"
#include "base/path_service.h"
#include "base/prefs/pref_service.h"
#include "base/stl_util.h"
Expand Down Expand Up @@ -63,7 +62,7 @@ struct InvalidatedGalleriesInfo {

} // namespace

MediaFileSystemInfo::MediaFileSystemInfo(const std::string& fs_name,
MediaFileSystemInfo::MediaFileSystemInfo(const string16& fs_name,
const base::FilePath& fs_path,
const std::string& filesystem_id,
MediaGalleryPrefId pref_id,
Expand Down Expand Up @@ -357,7 +356,7 @@ class ExtensionGalleriesHost
DCHECK(!fsid.empty());

MediaFileSystemInfo new_entry(
MakeJSONFileSystemName(gallery_info.display_name, pref_id, device_id),
gallery_info.display_name,
path,
fsid,
pref_id,
Expand Down Expand Up @@ -391,46 +390,6 @@ class ExtensionGalleriesHost
return monitor->GetTransientIdForDeviceId(device_id);
}

// This code is deprecated and should be removed. See http://crbug.com/170138
// Make a JSON string out of |name|, |pref_id| and |device_id|. The IDs makes
// the combined name unique. The JSON string should not contain any slashes.
std::string MakeJSONFileSystemName(const string16& name,
const MediaGalleryPrefId& pref_id,
const std::string& device_id) {
string16 sanitized_name;
string16 separators =
#if defined(FILE_PATH_USES_WIN_SEPARATORS)
base::FilePath::kSeparators
#else
ASCIIToUTF16(base::FilePath::kSeparators)
#endif
; // NOLINT
ReplaceChars(name, separators.c_str(), ASCIIToUTF16("_"), &sanitized_name);

base::DictionaryValue dict_value;
dict_value.SetStringWithoutPathExpansion(
MediaFileSystemRegistry::kNameKey, sanitized_name);

// This should have been a StringValue, but it's a bit late to change it.
dict_value.SetIntegerWithoutPathExpansion(
MediaFileSystemRegistry::kGalleryIdKey, pref_id);

// |device_id| can be empty, in which case, just omit it.
std::string transient_device_id =
GetTransientIdForRemovableDeviceId(device_id);
if (!transient_device_id.empty()) {
dict_value.SetStringWithoutPathExpansion(
MediaFileSystemRegistry::kDeviceIdKey, transient_device_id);
}
dict_value.SetStringWithoutPathExpansion(
"DEPRECATED",
"This JSON string is deprecated, use getMediaFileSystemMetadata.");

std::string json_string;
base::JSONWriter::Write(&dict_value, &json_string);
return json_string;
}

void CleanUp() {
DCHECK(rph_refs_.empty());
for (PrefIdFsInfoMap::const_iterator it = pref_id_map_.begin();
Expand Down Expand Up @@ -474,10 +433,6 @@ class ExtensionGalleriesHost
* Public methods
******************/

const char MediaFileSystemRegistry::kDeviceIdKey[] = "deviceId";
const char MediaFileSystemRegistry::kGalleryIdKey[] = "galleryId";
const char MediaFileSystemRegistry::kNameKey[] = "name";

void MediaFileSystemRegistry::GetMediaFileSystemsForExtension(
const content::RenderViewHost* rvh,
const extensions::Extension* extension,
Expand Down
9 changes: 2 additions & 7 deletions chrome/browser/media_galleries/media_file_system_registry.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class MediaGalleriesPreferences;
class ScopedMTPDeviceMapEntry;

struct MediaFileSystemInfo {
MediaFileSystemInfo(const std::string& fs_name,
MediaFileSystemInfo(const string16& fs_name,
const base::FilePath& fs_path,
const std::string& filesystem_id,
MediaGalleryPrefId pref_id,
Expand All @@ -57,7 +57,7 @@ struct MediaFileSystemInfo {
MediaFileSystemInfo();
~MediaFileSystemInfo();

std::string name; // JSON string, must not contain slashes.
string16 name;
base::FilePath path;
std::string fsid;
MediaGalleryPrefId pref_id;
Expand Down Expand Up @@ -96,11 +96,6 @@ class MediaFileSystemRegistry : public RemovableStorageObserver {
// See TransientDeviceIds::GetTransientIdForDeviceId().
uint64 GetTransientIdForDeviceId(const std::string& device_id);

// Keys for metadata about a media gallery file system.
static const char kDeviceIdKey[];
static const char kGalleryIdKey[];
static const char kNameKey[];

private:
friend class TestMediaFileSystemContext;
class MediaFileSystemContextImpl;
Expand Down
51 changes: 16 additions & 35 deletions chrome/browser/media_galleries/media_file_system_registry_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "base/command_line.h"
#include "base/file_util.h"
#include "base/files/scoped_temp_dir.h"
#include "base/json/json_reader.h"
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/scoped_vector.h"
Expand Down Expand Up @@ -188,39 +187,17 @@ void GetGalleryInfoCallback(
}
}

void CheckGalleryJSONName(const std::string& name, bool removable) {
scoped_ptr<DictionaryValue> dict(static_cast<DictionaryValue*>(
base::JSONReader::Read(name)));
ASSERT_TRUE(dict);

// Check deviceId.
EXPECT_EQ(removable,
dict->HasKey(MediaFileSystemRegistry::kDeviceIdKey)) << name;
if (removable) {
std::string device_id;
EXPECT_TRUE(dict->GetString(MediaFileSystemRegistry::kDeviceIdKey,
&device_id)) << name;
EXPECT_FALSE(device_id.empty()) << name;
}

// Check galleryId.
EXPECT_TRUE(dict->HasKey(MediaFileSystemRegistry::kGalleryIdKey)) << name;

// Check name.
EXPECT_TRUE(dict->HasKey(MediaFileSystemRegistry::kNameKey)) << name;
std::string gallery_name;
EXPECT_TRUE(dict->GetString(MediaFileSystemRegistry::kNameKey,
&gallery_name)) << name;
EXPECT_FALSE(gallery_name.empty()) << name;
}

void CheckGalleryInfo(const MediaFileSystemInfo& info,
TestMediaFileSystemContext* fs_context,
const string16* name,
const base::FilePath& path,
bool removable,
bool media_device) {
// TODO(vandebo) check the name (from path.LossyDisplayName) after JSON
// removal.
if (name) {
EXPECT_EQ(*name, info.name);
} else {
EXPECT_EQ(path.LossyDisplayName(), info.name);
}
EXPECT_EQ(path, info.path);
EXPECT_EQ(removable, info.removable);
EXPECT_EQ(media_device, info.media_device);
Expand Down Expand Up @@ -322,6 +299,10 @@ class MediaFileSystemRegistryTest : public ChromeRenderViewHostTestHarness {
return dcim_dir_;
}

TestMediaFileSystemContext* test_file_system_context() {
return test_file_system_context_;
}

// Create a user added gallery based on the information passed and add it to
// |profiles|. Returns the device id.
std::string AddUserGallery(MediaStorageUtil::Type type,
Expand Down Expand Up @@ -711,9 +692,8 @@ void MediaFileSystemRegistryTest::CheckNewGalleryInfo(
continue;

ASSERT_FALSE(found_new);
CheckGalleryJSONName(it->second.name, removable);
CheckGalleryInfo(it->second, test_file_system_context_, location, removable,
media_device);
CheckGalleryInfo(it->second, test_file_system_context_, NULL, location,
removable, media_device);
found_new = true;
}
ASSERT_TRUE(found_new);
Expand All @@ -730,7 +710,7 @@ MediaFileSystemRegistryTest::GetAutoAddedGalleries(
++it) {
if (it->second.type == MediaGalleryPrefInfo::kAutoDetected) {
base::FilePath path = it->second.AbsolutePath();
MediaFileSystemInfo info(path.AsUTF8Unsafe(), path, std::string(),
MediaFileSystemInfo info(path.LossyDisplayName(), path, std::string(),
0, std::string(), false, false);
result.push_back(info);
}
Expand Down Expand Up @@ -811,7 +791,7 @@ TEST_F(MediaFileSystemRegistryTest, UserAddedGallery) {
profile_state->regular_permission_extension(),
device_id,
true /*has access*/);
MediaFileSystemInfo added_info(empty_dir().AsUTF8Unsafe(), empty_dir(),
MediaFileSystemInfo added_info(empty_dir().LossyDisplayName(), empty_dir(),
std::string(), 0, std::string(), false, false);
added_galleries.push_back(added_info);
profile_state->CheckGalleries("user added regular", added_galleries,
Expand Down Expand Up @@ -891,7 +871,8 @@ TEST_F(MediaFileSystemRegistryTest, GalleryNameDefault) {
for (FSInfoMap::const_iterator it = galleries_info.begin();
it != galleries_info.end();
++it) {
CheckGalleryJSONName(it->second.name, false /*not removable*/);
CheckGalleryInfo(it->second, test_file_system_context(), &it->second.name,
it->second.path, false, false);
}
}

Expand Down
17 changes: 4 additions & 13 deletions chrome/renderer/extensions/media_galleries_custom_bindings.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,38 +31,29 @@ MediaGalleriesCustomBindings::MediaGalleriesCustomBindings(

v8::Handle<v8::Value> MediaGalleriesCustomBindings::GetMediaFileSystemObject(
const v8::Arguments& args) {
if (args.Length() != 2) {
if (args.Length() != 1) {
NOTREACHED();
return v8::Undefined();
}
if (!args[0]->IsString()) {
NOTREACHED();
return v8::Undefined();
}
if (!args[1]->IsString()) {
NOTREACHED();
return v8::Undefined();
}

std::string fsid(*v8::String::Utf8Value(args[0]));
if (fsid.empty()) {
NOTREACHED();
return v8::Undefined();
}
std::string name(*v8::String::Utf8Value(args[1]));
if (name.empty()) {
NOTREACHED();
return v8::Undefined();
}

WebKit::WebFrame* webframe = WebKit::WebFrame::frameForCurrentContext();
const GURL origin = GURL(webframe->document().securityOrigin().toString());
const std::string fs_name = fileapi::GetIsolatedFileSystemName(origin, fsid);
const std::string root_url =
fileapi::GetIsolatedFileSystemRootURIString(
origin, fsid, extension_misc::kMediaFileSystemPathPart);
return webframe->createFileSystem(
WebKit::WebFileSystemTypeIsolated,
WebKit::WebString::fromUTF8(name),
return webframe->createFileSystem(WebKit::WebFileSystemTypeIsolated,
WebKit::WebString::fromUTF8(fs_name),
WebKit::WebString::fromUTF8(root_url));
}

Expand Down
5 changes: 1 addition & 4 deletions chrome/renderer/resources/extensions/media_galleries_custom_bindings.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,10 @@ binding.registerCustomHook(function(bindingsAPI, extensionId) {
result = [];
for (var i = 0; i < response.length; i++) {
var filesystem = mediaGalleriesNatives.GetMediaFileSystemObject(
response[i].fsid, response[i].name);
response[i].fsid);
result.push(filesystem);
var metadata = response[i];
delete metadata.fsid;
// TODO(vandebo) Until we remove the JSON name string (one release), we
// need to pull out the real name part for metadata.
metadata.name = JSON.parse(metadata.name).name;
mediaGalleriesMetadata[filesystem.name] = metadata;
}
}
Expand Down

0 comments on commit fca4da9

Please sign in to comment.