Postpone setting up file handler's file permissions if handler is running lazy background page.

chrome/browser/chromeos/extensions/file_handler_util.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.
 
 #include "chrome/browser/chromeos/extensions/file_handler_util.h"
 
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/i18n/case_conversion.h"
 #include "base/json/json_writer.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/chromeos/gdata/gdata_util.h"
 #include "chrome/browser/chromeos/extensions/file_manager_util.h"
 #include "chrome/browser/extensions/extension_event_router.h"
+#include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/file_browser_handler.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/child_process_security_policy.h"
+#include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/web_contents.h"
 #include "net/base/escape.h"
 #include "webkit/fileapi/file_system_context.h"
 #include "webkit/fileapi/file_system_mount_point_provider.h"
 #include "webkit/fileapi/file_system_util.h"
 
 using content::BrowserContext;
 using content::BrowserThread;
@@ skipping 406 matching lines @@
     // If the file is under gdata mount point, there is no actual file to be
     // found on the final_file_path.
     if (!is_gdata_file) {
       if (!file_util::PathExists(final_file_path) ||
           file_util::IsLink(final_file_path) ||
           !file_util::GetFileInfo(final_file_path, &file_info)) {
         return false;
       }
     }
 
-    ChildProcessSecurityPolicy::GetInstance()->GrantPermissionsForFile(
-        handler_pid_,
-        final_file_path,
-        GetAccessPermissionsForHandler(handler_extension_.get(), action_id_));
-
     // Grant access to this particular file to target extension. This will
     // ensure that the target extension can access only this FS entry and
     // prevent from traversing FS hierarchy upward.
     external_provider->GrantFileAccessToExtension(handler_extension_->id(),
                                                   virtual_path);
 
     // Output values.
     GURL target_origin_url(Extension::GetBaseURLFromExtensionId(
         handler_extension_->id()));
     GURL base_url = fileapi::GetFileSystemRootURI(target_origin_url,
@@ skipping 17 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(ExecuteTasksFileSystemCallbackDispatcher);
 };
 
 FileTaskExecutor::FileTaskExecutor(Profile* profile,
                                    const GURL source_url,
                                    const std::string& extension_id,
                                    const std::string& action_id)
   : profile_(profile),
     source_url_(source_url),
     extension_id_(extension_id),
-    action_id_(action_id)
-{}
+    action_id_(action_id) {
+}
 
 FileTaskExecutor::~FileTaskExecutor() {}
 
 bool FileTaskExecutor::Execute(const std::vector<GURL>& file_urls) {
   ExtensionService* service = profile_->GetExtensionService();
   if (!service)
     return false;
 
   scoped_refptr<const Extension> handler =
       service->GetExtensionById(extension_id_, false);
 
   if (!handler.get())
     return false;
 
   int handler_pid = ExtractProcessFromExtensionId(handler->id(), profile_);
-  if (handler_pid < 0)
-    return false;
+  if (handler_pid <= 0) {
+    if (!handler->has_lazy_background_page())
+      return false;
+    RegisterNotificationObservers();

[Matt Perry, 2012/04/13 18:45:17]

Now that I think about it, this bit is dangerous. If the extension is not
actually listening to the event you are about to dispatch, we will never load
the background page, and this object will wait forever and leak.

I know I advised you against using the LazyBackgroundTaskQueue directly, but I
think it's preferable in this case. For one, it will take a reference to the
Executor, so that you don't need to keep a member var in
ExecuteTasksFileBrowserFunction. Second, it will handle failure cases for you*,
so you don't have to worry about the extension unloading or the page crashing.

Sorry to change my mind on this after the fact...

* Right now you register a callback with LazyBackgroundTaskQueue, and it will
call the callback only if the transient page successfully loads. The callback is
deleted if anything fails. If you can handle the failure case in the Executor
destructor, that should work fine. Otherwise, I can change the LBTQ to invoke a
callback on failure as well, so you can do cleanup.

[Matt Perry, 2012/04/13 20:29:07]

FYI, I have http://codereview.chromium.org/10082019/ in case you need to do
cleanup when the transient page fails to load.

+  }
 
   // Get local file system instance on file thread.
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(
           &FileTaskExecutor::RequestFileEntryOnFileThread,
           this,
           Extension::GetBaseURLFromExtensionId(handler->id()),
           handler,
           handler_pid,
@@ skipping 17 matching lines @@
           handler,
           handler_pid,
           action_id_,
           file_urls));
 }
 
 void FileTaskExecutor::ExecuteFailedOnUIThread() {
   Done(false);
 }
 
-void FileTaskExecutor::SetupFileAccessPermissionsForGDataCache(
-    const FileDefinitionList& file_list,
-    int handler_pid) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  for (FileDefinitionList::const_iterator iter = file_list.begin();
-       iter != file_list.end();
-       ++iter) {
-    if (!gdata::util::IsUnderGDataMountPoint(iter->absolute_path))
-      continue;
-    gdata::util::SetPermissionsForGDataCacheFiles(profile_, handler_pid,
-        iter->absolute_path);
-  }
-}
-
 void FileTaskExecutor::ExecuteFileActionsOnUIThread(
     const std::string& file_system_name,
     const GURL& file_system_root,
     const FileDefinitionList& file_list,
     int handler_pid) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   ExtensionService* service = profile_->GetExtensionService();
   if (!service) {
     Done(false);
     return;
   }
 
   const Extension* extension = service->GetExtensionById(extension_id_, false);
   if (!extension) {
     Done(false);
     return;
   }
 
   ExtensionEventRouter* event_router = profile_->GetExtensionEventRouter();
   if (!event_router) {
     Done(false);
     return;
   }
 
-  SetupFileAccessPermissionsForGDataCache(file_list, handler_pid);
+  InitHandlerHostFileAccessPermissions(file_list, extension, action_id_);
+
+  if (handler_pid > 0)
+    SetupHandlerHostFileAccessPermissions(handler_pid);
 
   scoped_ptr<ListValue> event_args(new ListValue());
   event_args->Append(Value::CreateStringValue(action_id_));
   DictionaryValue* details = new DictionaryValue();
   event_args->Append(details);
   // Get file definitions. These will be replaced with Entry instances by
   // chromeHidden.Event.dispatchJSON() method from even_binding.js.
   ListValue* files_urls = new ListValue();
   details->Set("entries", files_urls);
   for (FileDefinitionList::const_iterator iter = file_list.begin();
@@ skipping 15 matching lines @@
   }
 
   UpdateFileHandlerUsageStats(profile_, MakeTaskID(extension_id_, action_id_));
 
   std::string json_args;
   base::JSONWriter::Write(event_args.get(), &json_args);
   event_router->DispatchEventToExtension(
       extension_id_, std::string("fileBrowserHandler.onExecute"),
       json_args, profile_,
       GURL());
-  Done(true);
+
+  // If we don't have handler process id, we'll have to wait until the
+  // background host loads so we can setup file access permissions.
+  if (handler_pid > 0)
+    Done(true);
+}
+
+void FileTaskExecutor::InitHandlerHostFileAccessPermissions(
+    const FileDefinitionList& file_list,
+    const Extension* handler_extension,
+    const std::string& action_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  for (FileDefinitionList::const_iterator iter = file_list.begin();
+       iter != file_list.end();
+       ++iter) {
+    // Setup permission for file's absolute file.
+    handler_host_permissions_.push_back(std::make_pair(
+        iter->absolute_path,
+        GetAccessPermissionsForHandler(handler_extension, action_id)));
+
+    if (!gdata::util::IsUnderGDataMountPoint(iter->absolute_path))
+      continue;
+
+    // If the file is on gdata mount point, we'll have to give handler host
+    // permissions for file's gdata cache paths.
+    // This has to be called on UI thread.
+    gdata::util::InsertGDataCachePathsPermissions(profile_, iter->absolute_path,
+        &handler_host_permissions_);
+  }
+}
+
+void FileTaskExecutor::SetupHandlerHostFileAccessPermissions(int handler_pid) {
+  for (size_t i = 0; i < handler_host_permissions_.size(); i++) {
+    content::ChildProcessSecurityPolicy::GetInstance()->GrantPermissionsForFile(
+        handler_pid,
+        handler_host_permissions_[i].first,
+        handler_host_permissions_[i].second);
+  }
+
+  // We don't need this anymore.
+  handler_host_permissions_.clear();
+}
+
+void FileTaskExecutor::RegisterNotificationObservers() {
+  // We should do this only once.
+  DCHECK(registrar_.IsEmpty());
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING,
+                 content::NotificationService::AllBrowserContextsAndSources());
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
+                 content::Source<Profile>(profile_));
+}
+
+void FileTaskExecutor::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  switch (type) {
+    case chrome::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING: {
+      ExtensionHost* host = content::Details<ExtensionHost>(details).ptr();
+      if (host->profile()->IsSameProfile(profile_) &&
+          host->extension()->id() == extension_id_) {
+        CHECK(host->did_stop_loading());
+        DCHECK(host->extension()->has_lazy_background_page());
+
+        SetupHandlerHostFileAccessPermissions(
+            host->render_process_host()->GetID());
+        Done(true);
+        break;
+      }
+    }
+    case chrome::NOTIFICATION_EXTENSION_UNLOADED: {
+      UnloadedExtensionInfo* unloaded =
+          content::Details<UnloadedExtensionInfo>(details).ptr();
+      if (unloaded->extension->id() == extension_id_) {
+        Done(false);
+        break;
+      }
+    }
+    default:
+      NOTREACHED();
+      break;
+  }
 }
 
 } // namespace file_handler_util