Commit a1568366 authored by mtomasz's avatar mtomasz Committed by Commit bot

[fsp] Pass proper data to Notify().

This patch passes proper data instead of fake one to Notify(). Also, child
changes are now passed as a scoped_ptr and a reference to avoid unnecessary
copies.

Note, that end to end browser tests will come once notifications are wired to
chrome.fileManagerPrivate.

TEST=Tested manually.
BUG=248427

Review URL: https://codereview.chromium.org/656393002

Cr-Commit-Position: refs/heads/master@{#300062}
parent 5592d099
...@@ -21,16 +21,60 @@ ...@@ -21,16 +21,60 @@
using chromeos::file_system_provider::MountOptions; using chromeos::file_system_provider::MountOptions;
using chromeos::file_system_provider::ProvidedFileSystemInfo; using chromeos::file_system_provider::ProvidedFileSystemInfo;
using chromeos::file_system_provider::ProvidedFileSystemInterface; using chromeos::file_system_provider::ProvidedFileSystemInterface;
using chromeos::file_system_provider::ProvidedFileSystemObserver;
using chromeos::file_system_provider::RequestValue; using chromeos::file_system_provider::RequestValue;
using chromeos::file_system_provider::Service; using chromeos::file_system_provider::Service;
namespace extensions { namespace extensions {
namespace { namespace {
typedef std::vector<linked_ptr<api::file_system_provider::ChildChange>>
IDLChildChanges;
const char kNotifyFailedErrorMessage[] = const char kNotifyFailedErrorMessage[] =
"Sending a response for the request failed."; "Sending a response for the request failed.";
const char kInvalidNotificationErrorMessage[] = "The notification is invalid."; const char kInvalidNotificationErrorMessage[] = "The notification is invalid.";
// Converts the change type from the IDL type to a native type. |changed_type|
// must be specified (not CHANGE_TYPE_NONE).
ProvidedFileSystemObserver::ChangeType ParseChangeType(
const api::file_system_provider::ChangeType& change_type) {
switch (change_type) {
case api::file_system_provider::CHANGE_TYPE_CHANGED:
return ProvidedFileSystemObserver::CHANGED;
case api::file_system_provider::CHANGE_TYPE_DELETED:
return ProvidedFileSystemObserver::DELETED;
default:
break;
}
NOTREACHED();
return ProvidedFileSystemObserver::CHANGED;
}
// Convert the child change from the IDL type to a native type. The reason IDL
// types are not used is since they are imperfect, eg. paths are stored as
// strings.
ProvidedFileSystemObserver::ChildChange ParseChildChange(
const api::file_system_provider::ChildChange& child_change) {
ProvidedFileSystemObserver::ChildChange result;
result.entry_path = base::FilePath::FromUTF8Unsafe(child_change.entry_path);
result.change_type = ParseChangeType(child_change.change_type);
return result;
}
// Converts a list of child changes from the IDL type to a native type.
scoped_ptr<ProvidedFileSystemObserver::ChildChanges> ParseChildChanges(
const IDLChildChanges& child_changes) {
scoped_ptr<ProvidedFileSystemObserver::ChildChanges> results(
new ProvidedFileSystemObserver::ChildChanges);
for (IDLChildChanges::const_iterator it = child_changes.begin();
it != child_changes.end();
++it) {
results->push_back(ParseChildChange(*it->get()));
}
return results;
}
} // namespace } // namespace
bool FileSystemProviderMountFunction::RunSync() { bool FileSystemProviderMountFunction::RunSync() {
...@@ -149,13 +193,13 @@ bool FileSystemProviderNotifyFunction::RunSync() { ...@@ -149,13 +193,13 @@ bool FileSystemProviderNotifyFunction::RunSync() {
return true; return true;
} }
// TODO(mtomasz): Pass real data to Notify() instead of fake ones.
if (!file_system->Notify( if (!file_system->Notify(
base::FilePath::FromUTF8Unsafe(params->options.observed_path), base::FilePath::FromUTF8Unsafe(params->options.observed_path),
chromeos::file_system_provider::ProvidedFileSystemObserver::CHANGED, ParseChangeType(params->options.change_type),
chromeos::file_system_provider::ProvidedFileSystemObserver:: params->options.child_changes.get()
ChildChanges(), ? ParseChildChanges(*params->options.child_changes.get())
"todo-tag")) { : make_scoped_ptr(new ProvidedFileSystemObserver::ChildChanges),
params->options.tag.get() ? *params->options.tag.get() : "")) {
base::ListValue* const result = new base::ListValue(); base::ListValue* const result = new base::ListValue();
result->Append( result->Append(
CreateError(kSecurityErrorName, kInvalidNotificationErrorMessage)); CreateError(kSecurityErrorName, kInvalidNotificationErrorMessage));
......
...@@ -131,4 +131,7 @@ IN_PROC_BROWSER_TEST_F(FileSystemProviderApiTest, Thumbnail) { ...@@ -131,4 +131,7 @@ IN_PROC_BROWSER_TEST_F(FileSystemProviderApiTest, Thumbnail) {
<< message_; << message_;
} }
// TODO(mtomasz): Add a test for Notify() once it's wired to
// chrome.fileManagerPrivate.
} // namespace extensions } // namespace extensions
...@@ -367,7 +367,7 @@ void FakeProvidedFileSystem::RemoveObserver( ...@@ -367,7 +367,7 @@ void FakeProvidedFileSystem::RemoveObserver(
bool FakeProvidedFileSystem::Notify( bool FakeProvidedFileSystem::Notify(
const base::FilePath& observed_path, const base::FilePath& observed_path,
ProvidedFileSystemObserver::ChangeType change_type, ProvidedFileSystemObserver::ChangeType change_type,
const ProvidedFileSystemObserver::ChildChanges& child_changes, scoped_ptr<ProvidedFileSystemObserver::ChildChanges> child_changes,
const std::string& tag) { const std::string& tag) {
NOTREACHED(); NOTREACHED();
return false; return false;
......
...@@ -139,7 +139,7 @@ class FakeProvidedFileSystem : public ProvidedFileSystemInterface { ...@@ -139,7 +139,7 @@ class FakeProvidedFileSystem : public ProvidedFileSystemInterface {
virtual bool Notify( virtual bool Notify(
const base::FilePath& observed_path, const base::FilePath& observed_path,
ProvidedFileSystemObserver::ChangeType change_type, ProvidedFileSystemObserver::ChangeType change_type,
const ProvidedFileSystemObserver::ChildChanges& child_changes, scoped_ptr<ProvidedFileSystemObserver::ChildChanges> child_changes,
const std::string& tag) override; const std::string& tag) override;
virtual base::WeakPtr<ProvidedFileSystemInterface> GetWeakPtr() override; virtual base::WeakPtr<ProvidedFileSystemInterface> GetWeakPtr() override;
......
...@@ -448,7 +448,7 @@ void ProvidedFileSystem::RemoveObserver(ProvidedFileSystemObserver* observer) { ...@@ -448,7 +448,7 @@ void ProvidedFileSystem::RemoveObserver(ProvidedFileSystemObserver* observer) {
bool ProvidedFileSystem::Notify( bool ProvidedFileSystem::Notify(
const base::FilePath& observed_path, const base::FilePath& observed_path,
ProvidedFileSystemObserver::ChangeType change_type, ProvidedFileSystemObserver::ChangeType change_type,
const ProvidedFileSystemObserver::ChildChanges& child_changes, scoped_ptr<ProvidedFileSystemObserver::ChildChanges> child_changes,
const std::string& tag) { const std::string& tag) {
const ObservedEntries::iterator it = observed_entries_.find(observed_path); const ObservedEntries::iterator it = observed_entries_.find(observed_path);
if (it == observed_entries_.end()) if (it == observed_entries_.end())
...@@ -458,11 +458,17 @@ bool ProvidedFileSystem::Notify( ...@@ -458,11 +458,17 @@ bool ProvidedFileSystem::Notify(
if (file_system_info_.supports_notify_tag() == tag.empty()) if (file_system_info_.supports_notify_tag() == tag.empty())
return false; return false;
// The object is owned by AutoUpdated, so the reference is valid as long as
// callbacks created with AutoUpdater::CreateCallback().
const ProvidedFileSystemObserver::ChildChanges& child_changes_ref =
*child_changes.get();
scoped_refptr<AutoUpdater> auto_updater( scoped_refptr<AutoUpdater> auto_updater(
new AutoUpdater(base::Bind(&ProvidedFileSystem::OnNotifyCompleted, new AutoUpdater(base::Bind(&ProvidedFileSystem::OnNotifyCompleted,
weak_ptr_factory_.GetWeakPtr(), weak_ptr_factory_.GetWeakPtr(),
observed_path, observed_path,
change_type, change_type,
base::Passed(&child_changes),
it->second.last_tag, it->second.last_tag,
tag))); tag)));
...@@ -471,7 +477,7 @@ bool ProvidedFileSystem::Notify( ...@@ -471,7 +477,7 @@ bool ProvidedFileSystem::Notify(
OnObservedEntryChanged(file_system_info_, OnObservedEntryChanged(file_system_info_,
observed_path, observed_path,
change_type, change_type,
child_changes, child_changes_ref,
auto_updater->CreateCallback())); auto_updater->CreateCallback()));
return true; return true;
...@@ -521,6 +527,7 @@ void ProvidedFileSystem::OnObserveDirectoryCompleted( ...@@ -521,6 +527,7 @@ void ProvidedFileSystem::OnObserveDirectoryCompleted(
void ProvidedFileSystem::OnNotifyCompleted( void ProvidedFileSystem::OnNotifyCompleted(
const base::FilePath& observed_path, const base::FilePath& observed_path,
ProvidedFileSystemObserver::ChangeType change_type, ProvidedFileSystemObserver::ChangeType change_type,
scoped_ptr<ProvidedFileSystemObserver::ChildChanges> /* child_changes */,
const std::string& last_tag, const std::string& last_tag,
const std::string& tag) { const std::string& tag) {
const ObservedEntries::iterator it = observed_entries_.find(observed_path); const ObservedEntries::iterator it = observed_entries_.find(observed_path);
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <string> #include <string>
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/observer_list.h" #include "base/observer_list.h"
#include "chrome/browser/chromeos/file_system_provider/provided_file_system_info.h" #include "chrome/browser/chromeos/file_system_provider/provided_file_system_info.h"
...@@ -150,7 +151,7 @@ class ProvidedFileSystem : public ProvidedFileSystemInterface { ...@@ -150,7 +151,7 @@ class ProvidedFileSystem : public ProvidedFileSystemInterface {
virtual bool Notify( virtual bool Notify(
const base::FilePath& observed_path, const base::FilePath& observed_path,
ProvidedFileSystemObserver::ChangeType change_type, ProvidedFileSystemObserver::ChangeType change_type,
const ProvidedFileSystemObserver::ChildChanges& child_changes, scoped_ptr<ProvidedFileSystemObserver::ChildChanges> child_changes,
const std::string& tag) override; const std::string& tag) override;
virtual base::WeakPtr<ProvidedFileSystemInterface> GetWeakPtr() override; virtual base::WeakPtr<ProvidedFileSystemInterface> GetWeakPtr() override;
...@@ -170,10 +171,12 @@ class ProvidedFileSystem : public ProvidedFileSystemInterface { ...@@ -170,10 +171,12 @@ class ProvidedFileSystem : public ProvidedFileSystemInterface {
// Called when all observers finished handling the change notification. It // Called when all observers finished handling the change notification. It
// updates the tag from |last_tag| to |tag| for the entry at |observed_path|. // updates the tag from |last_tag| to |tag| for the entry at |observed_path|.
void OnNotifyCompleted(const base::FilePath& observed_path, void OnNotifyCompleted(
ProvidedFileSystemObserver::ChangeType change_type, const base::FilePath& observed_path,
const std::string& last_tag, ProvidedFileSystemObserver::ChangeType change_type,
const std::string& tag); scoped_ptr<ProvidedFileSystemObserver::ChildChanges> child_changes,
const std::string& last_tag,
const std::string& tag);
Profile* profile_; // Not owned. Profile* profile_; // Not owned.
extensions::EventRouter* event_router_; // Not owned. May be NULL. extensions::EventRouter* event_router_; // Not owned. May be NULL.
......
...@@ -207,7 +207,7 @@ class ProvidedFileSystemInterface { ...@@ -207,7 +207,7 @@ class ProvidedFileSystemInterface {
virtual bool Notify( virtual bool Notify(
const base::FilePath& observed_path, const base::FilePath& observed_path,
ProvidedFileSystemObserver::ChangeType change_type, ProvidedFileSystemObserver::ChangeType change_type,
const ProvidedFileSystemObserver::ChildChanges& child_changes, scoped_ptr<ProvidedFileSystemObserver::ChildChanges> child_changes,
const std::string& tag) = 0; const std::string& tag) = 0;
// Returns a provided file system info for this file system. // Returns a provided file system info for this file system.
......
...@@ -51,7 +51,8 @@ class ProvidedFileSystemObserver { ...@@ -51,7 +51,8 @@ class ProvidedFileSystemObserver {
// Called when an observed entry is changed, including removals. |callback| // Called when an observed entry is changed, including removals. |callback|
// *must* be called after the entry change is handled. Once all observers // *must* be called after the entry change is handled. Once all observers
// call the callback, the tag will be updated and OnObservedEntryTagUpdated // call the callback, the tag will be updated and OnObservedEntryTagUpdated
// called. // called. The reference to |child_changes| is valid at least as long as
// |callback|.
virtual void OnObservedEntryChanged( virtual void OnObservedEntryChanged(
const ProvidedFileSystemInfo& file_system_info, const ProvidedFileSystemInfo& file_system_info,
const base::FilePath& observed_path, const base::FilePath& observed_path,
......
...@@ -539,12 +539,11 @@ TEST_F(FileSystemProviderProvidedFileSystemTest, Notify) { ...@@ -539,12 +539,11 @@ TEST_F(FileSystemProviderProvidedFileSystemTest, Notify) {
// Notify about a change. // Notify about a change.
const ProvidedFileSystemObserver::ChangeType change_type = const ProvidedFileSystemObserver::ChangeType change_type =
ProvidedFileSystemObserver::CHANGED; ProvidedFileSystemObserver::CHANGED;
const ProvidedFileSystemObserver::ChildChanges child_changes;
const std::string tag = "hello-world"; const std::string tag = "hello-world";
EXPECT_TRUE(provided_file_system_->Notify( EXPECT_TRUE(provided_file_system_->Notify(
base::FilePath::FromUTF8Unsafe(kDirectoryPath), base::FilePath::FromUTF8Unsafe(kDirectoryPath),
change_type, change_type,
child_changes, make_scoped_ptr(new ProvidedFileSystemObserver::ChildChanges),
tag)); tag));
// Verify the observer event. // Verify the observer event.
...@@ -582,7 +581,7 @@ TEST_F(FileSystemProviderProvidedFileSystemTest, Notify) { ...@@ -582,7 +581,7 @@ TEST_F(FileSystemProviderProvidedFileSystemTest, Notify) {
EXPECT_TRUE(provided_file_system_->Notify( EXPECT_TRUE(provided_file_system_->Notify(
base::FilePath::FromUTF8Unsafe(kDirectoryPath), base::FilePath::FromUTF8Unsafe(kDirectoryPath),
change_type, change_type,
child_changes, make_scoped_ptr(new ProvidedFileSystemObserver::ChildChanges),
tag)); tag));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment