Expose gallery watching functionality to the public mediaGalleries API.

chrome/browser/extensions/api/media_galleries/media_galleries_api.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.
 
 // Implements the Chrome Extensions Media Galleries API.
 
 #include "chrome/browser/extensions/api/media_galleries/media_galleries_api.h"
 
 #include <set>
 #include <string>
@@ skipping 50 matching lines @@
 using storage_monitor::MediaStorageUtil;
 using storage_monitor::StorageInfo;
 using web_modal::WebContentsModalDialogManager;
 
 namespace extensions {
 
 namespace MediaGalleries = api::media_galleries;
 namespace DropPermissionForMediaFileSystem =
     MediaGalleries::DropPermissionForMediaFileSystem;
 namespace GetMediaFileSystems = MediaGalleries::GetMediaFileSystems;
+namespace AddGalleryWatch =
+    extensions::api::media_galleries::AddGalleryWatch;

[tommycli, 2014/08/19 16:36:02]

make consistent with above declarations... i.e. MediaGalleries::AddGalleryWatch.
And below 2 also.

[Oren Blasberg, 2014/08/20 22:59:36]

Done.

+namespace RemoveGalleryWatch =
+    extensions::api::media_galleries::RemoveGalleryWatch;
+namespace GetAllGalleryWatch =
+    extensions::api::media_galleries::GetAllGalleryWatch;
 
 namespace {
 
 const char kDisallowedByPolicy[] =
     "Media Galleries API is disallowed by policy: ";
 const char kFailedToSetGalleryPermission[] =
     "Failed to set gallery permission.";
 const char kInvalidGalleryId[] = "Invalid gallery id.";
 const char kMissingEventListener[] = "Missing event listener registration.";
 const char kNonExistentGalleryId[] = "Non-existent gallery id.";
@@ skipping 33 matching lines @@
         prefs::kAllowFileSelectionDialogs;
     return false;
   }
 
   MediaGalleriesPreferences* preferences =
       media_file_system_registry()->GetPreferences(profile);
   preferences->EnsureInitialized(callback);
   return true;
 }
 
+// Gets the |gallery_file_path| and |gallery_pref_id| of the gallery specified

[tommycli, 2014/08/19 16:36:02]

Make briefer. Probably don't need both first sentence and second. Should be
"Returns true and sets..." if you choose to use the second sentence.

Probably update the comment in the private api too.

[Oren Blasberg, 2014/08/20 22:59:36]

Done.

+// by the |gallery_id|. Returns true and set |gallery_file_path| and
+// |gallery_pref_id| if the |gallery_id| is valid and returns false otherwise.
+bool GetGalleryFilePathAndId(const std::string& gallery_id,
+                             Profile* profile,
+                             const Extension* extension,
+                             base::FilePath* gallery_file_path,
+                             MediaGalleryPrefId* gallery_pref_id) {
+  MediaGalleryPrefId pref_id;
+  if (!base::StringToUint64(gallery_id, &pref_id))
+    return false;
+  MediaGalleriesPreferences* preferences =
+      g_browser_process->media_file_system_registry()->GetPreferences(profile);
+  base::FilePath file_path(
+      preferences->LookUpGalleryPathForExtension(pref_id, extension, false));
+  if (file_path.empty())
+    return false;
+  *gallery_pref_id = pref_id;
+  *gallery_file_path = file_path;
+  return true;
+}
+
 WebContents* GetWebContents(content::RenderViewHost* rvh,
                             Profile* profile,
                             const std::string& app_id) {
   WebContents* contents = WebContents::FromRenderViewHost(rvh);
   WebContentsModalDialogManager* web_contents_modal_dialog_manager =
       WebContentsModalDialogManager::FromWebContents(contents);
   if (!web_contents_modal_dialog_manager) {
     // If there is no WebContentsModalDialogManager, then this contents is
     // probably the background page for an app. Try to find a app window to
     // host the dialog.
@@ skipping 893 matching lines @@
                    base::Passed(&attached_images), base::Passed(&blob_uuids)));
     return;
   }
 
   // All Blobs have been constructed. The renderer will take ownership.
   SetResult(result_dictionary.release());
   SetTransferredBlobUUIDs(*blob_uuids);
   SendResponse(true);
 }
 
+///////////////////////////////////////////////////////////////////////////////

[tommycli, 2014/08/19 16:36:02]

We don't use this block to divide between classes in this file. But I do think
it's a good idea. maybe add them for the other classes?

[Oren Blasberg, 2014/08/20 22:59:36]

Done.

+//              MediaGalleriesAddGalleryWatchFunction                        //
+///////////////////////////////////////////////////////////////////////////////
+MediaGalleriesAddGalleryWatchFunction::

[tommycli, 2014/08/19 16:36:02]

I assume you ran git cl format on everything right? If you did, I will say
nothing about formatting whatsoever...

[Oren Blasberg, 2014/08/20 22:59:36]

Didn't at first, did in patch 4.

+~MediaGalleriesAddGalleryWatchFunction() {
+}
+
+bool MediaGalleriesAddGalleryWatchFunction::RunAsync() {
+  DCHECK(GetProfile());
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (!render_view_host() || !render_view_host()->GetProcess())
+    return false;
+
+  scoped_ptr<AddGalleryWatch::Params> params(
+      AddGalleryWatch::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  MediaGalleriesPreferences* preferences =
+      g_browser_process->media_file_system_registry()->GetPreferences(
+          GetProfile());
+  preferences->EnsureInitialized(base::Bind(
+      &MediaGalleriesAddGalleryWatchFunction::OnPreferencesInit,
+      this,
+      params->gallery_id));
+
+  return true;
+}
+
+void MediaGalleriesAddGalleryWatchFunction::OnPreferencesInit(
+    const std::string& pref_id) {
+
+  base::FilePath gallery_file_path;
+  MediaGalleryPrefId gallery_pref_id = 0;
+  if (!GetGalleryFilePathAndId(pref_id,
+                               GetProfile(),
+                               extension(),
+                               &gallery_file_path,
+                               &gallery_pref_id)) {
+    error_ = kInvalidGalleryId;
+    const std::string error_str(kInvalidGalleryId);
+    HandleResponse(gallery_pref_id, error_str);

[tommycli, 2014/08/19 16:36:02]

If you have an error here, you still check for an event listener in the
HandleResponse section... you should probably just return an error
immediately... 

These two errors are handled inconsistently. In one, you set a result, and in
the other, you don't.

[Oren Blasberg, 2014/08/20 22:59:36]

Done.

+    return;
+  }
+
+  gallery_watch_manager()->AddWatch(
+      GetProfile(),
+      extension(),
+      gallery_pref_id,
+      base::Bind(&MediaGalleriesAddGalleryWatchFunction::HandleResponse,
+                 this,
+                 gallery_pref_id));
+
+}
+
+void MediaGalleriesAddGalleryWatchFunction::HandleResponse(
+    MediaGalleryPrefId gallery_id,
+    const std::string& error) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // If an app added a file watch without any event listeners on the
+  // onGalleryChanged event, that's an error.
+  MediaGalleriesEventRouter* api = MediaGalleriesEventRouter::Get(GetProfile());
+  if (!api->ExtensionHasGalleryChangeListener(extension()->id())) {
+    api::media_galleries::AddGalleryWatchResult result;

[tommycli, 2014/08/19 16:36:02]

Does the callback still get called when there's an error?

[Oren Blasberg, 2014/08/20 22:59:36]

It does, as evidenced by the setupWatchOnUnlistenedValidGalleries test.
Also I tried it. It does, with success false, and the error occurs too.

[tommycli, 2014/08/20 23:21:12]

Awesome. Thanks for testing that out.

+    result.gallery_id = base::Uint64ToString(gallery_id);
+    result.success = false;
+    SetResult(result.ToValue().release());
+    error_ = kMissingEventListener;
+    SendResponse(false);
+    return;
+  }
+
+  api::media_galleries::AddGalleryWatchResult result;
+  result.gallery_id = base::Uint64ToString(gallery_id);
+  result.success = error.empty();
+  SetResult(result.ToValue().release());
+  if (error.empty()) {
+    base::DictionaryValue* dv = new base::DictionaryValue();
+    results_->GetDictionary(0, &dv);

[tommycli, 2014/08/19 16:36:02]

This looks strange. What's the purpose of this?

[Oren Blasberg, 2014/08/20 22:59:36]

Whoops. It was part of debugging code I forgot to remove.

+    SendResponse(true);
+  } else {
+    error_ = error.c_str();
+    SendResponse(false);
+  }
+}
+
+///////////////////////////////////////////////////////////////////////////////
+//              MediaGalleriesRemoveGalleryWatchFunction                     //
+///////////////////////////////////////////////////////////////////////////////
+
+MediaGalleriesRemoveGalleryWatchFunction::
+~MediaGalleriesRemoveGalleryWatchFunction() {
+}
+
+bool MediaGalleriesRemoveGalleryWatchFunction::RunAsync() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (!render_view_host() || !render_view_host()->GetProcess())
+    return false;
+
+  scoped_ptr<RemoveGalleryWatch::Params> params(
+      RemoveGalleryWatch::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  MediaGalleriesPreferences* preferences =
+      g_browser_process->media_file_system_registry()->GetPreferences(
+          GetProfile());
+  preferences->EnsureInitialized(base::Bind(
+      &MediaGalleriesRemoveGalleryWatchFunction::OnPreferencesInit,
+      this,
+      params->gallery_id));
+  return true;
+}
+
+void MediaGalleriesRemoveGalleryWatchFunction::OnPreferencesInit(
+    const std::string& pref_id) {
+  base::FilePath gallery_file_path;
+  MediaGalleryPrefId gallery_pref_id = 0;
+  if (!GetGalleryFilePathAndId(pref_id,
+                               GetProfile(),
+                               extension(),
+                               &gallery_file_path,
+                               &gallery_pref_id)) {
+    error_ = kInvalidGalleryId;
+    SendResponse(false);
+    return;
+  }
+
+  gallery_watch_manager()->RemoveWatch(
+      GetProfile(), extension_id(), gallery_pref_id);
+  SendResponse(true);
+}
+
+///////////////////////////////////////////////////////////////////////////////
+//              MediaGalleriesGetAllGalleryWatchFunction                     //
+///////////////////////////////////////////////////////////////////////////////
+
+MediaGalleriesGetAllGalleryWatchFunction::
+~MediaGalleriesGetAllGalleryWatchFunction() {
+}
+
+bool MediaGalleriesGetAllGalleryWatchFunction::RunAsync() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (!render_view_host() || !render_view_host()->GetProcess())
+    return false;
+
+  MediaGalleriesPreferences* preferences =
+      g_browser_process->media_file_system_registry()->GetPreferences(
+          GetProfile());
+  preferences->EnsureInitialized(base::Bind(
+      &MediaGalleriesGetAllGalleryWatchFunction::OnPreferencesInit,
+      this));
+  return true;
+}
+
+void MediaGalleriesGetAllGalleryWatchFunction::OnPreferencesInit() {
+  std::vector<std::string> result;
+  MediaGalleryPrefIdSet gallery_ids =
+      gallery_watch_manager()->GetWatchSet(GetProfile(), extension_id());
+  for (MediaGalleryPrefIdSet::const_iterator iter = gallery_ids.begin();
+       iter != gallery_ids.end(); ++iter) {
+    result.push_back(base::Uint64ToString(*iter));
+  }
+  results_ = GetAllGalleryWatch::Results::Create(result);

[tommycli, 2014/08/19 16:36:02]

I wonder if results_ = or SetResult is the correct thing here. They have
different meaning.

It's worth testing this.

[Oren Blasberg, 2014/08/20 22:59:36]

It looks like SetResult is used when you have just one Value:
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...

It's a convenience for making it into a ListValue.

In this case, we already have a list so it shouldn't be needed and setting
results_ directly is the most direct.

[tommycli, 2014/08/20 23:21:12]

Oh okay, great. thanks for verifyincg

+  SendResponse(true);
+}
+
+///////////////////////////////////////////////////////////////////////////////
+//              MediaGalleriesRemoveAllGalleryWatchFunction                  //
+///////////////////////////////////////////////////////////////////////////////
+
+MediaGalleriesRemoveAllGalleryWatchFunction::
+~MediaGalleriesRemoveAllGalleryWatchFunction() {
+}
+
+bool MediaGalleriesRemoveAllGalleryWatchFunction::RunAsync() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (!render_view_host() || !render_view_host()->GetProcess())
+    return false;
+
+  MediaGalleriesPreferences* preferences =
+      g_browser_process->media_file_system_registry()->GetPreferences(
+          GetProfile());
+  preferences->EnsureInitialized(base::Bind(
+      &MediaGalleriesRemoveAllGalleryWatchFunction::OnPreferencesInit,
+      this));
+  return true;
+}
+
+void MediaGalleriesRemoveAllGalleryWatchFunction::OnPreferencesInit() {
+  gallery_watch_manager()->RemoveAllWatches(
+      GetProfile(), extension_id());
+  SendResponse(true);
+}
+
 }  // namespace extensions