Skip to content

Commit

Permalink
Display the list of retained files in the extension permissions dialog.
Browse files Browse the repository at this point in the history
The dialog allows the user to revoke access for the app, restarting it
if it's currently running. If the app/extension has not retained access
to any files, the dialog is unchanged.

[Mac] XIB change:
- Enable "Show Vertical Scroller" for the scroll view.

Dialog screenshots: https://code.google.com/p/chromium/issues/detail?id=224684#c6

BUG=224684
TEST=Install an app that uses chrome.fileSystem.retainEntry and open a
file with it. In chrome://extensions/ click the permissions link for
that app. The filename should be listed.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@205028 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
sammc@chromium.org committed Jun 8, 2013
1 parent 7a57fe9 commit a2886e8
Show file tree
Hide file tree
Showing 16 changed files with 307 additions and 33 deletions.
11 changes: 11 additions & 0 deletions apps/app_restore_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "apps/app_restore_service.h"

#include "apps/app_restore_service_factory.h"
#include "apps/saved_files_service.h"
#include "chrome/browser/extensions/api/app_runtime/app_runtime_api.h"
#include "chrome/browser/extensions/event_router.h"
Expand Down Expand Up @@ -84,6 +85,16 @@ void AppRestoreService::HandleStartup(bool should_restore_apps) {
}
}

bool AppRestoreService::IsAppRestorable(const std::string& extension_id) {
return extensions::ExtensionPrefs::Get(profile_) ->IsExtensionRunning(
extension_id);
}

// static
AppRestoreService* AppRestoreService::Get(Profile* profile) {
return apps::AppRestoreServiceFactory::GetForProfile(profile);
}

void AppRestoreService::Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
Expand Down
6 changes: 6 additions & 0 deletions apps/app_restore_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ class AppRestoreService : public BrowserContextKeyedService,
// from apps to prevent them being restarted in subsequent restarts.
void HandleStartup(bool should_restore_apps);

// Returns whether this extension is running or, at startup, whether it was
// running when Chrome was last terminated.
bool IsAppRestorable(const std::string& extension_id);

static AppRestoreService* Get(Profile* profile);

private:
// content::NotificationObserver.
virtual void Observe(int type,
Expand Down
30 changes: 23 additions & 7 deletions apps/saved_files_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,10 @@ void SavedFilesService::EnqueueFileEntry(const std::string& extension_id,

std::vector<SavedFileEntry> SavedFilesService::GetAllFileEntries(
const std::string& extension_id) {
return GetOrInsert(extension_id)->GetAllFileEntries();
SavedFiles* saved_files = Get(extension_id);
if (saved_files)
return saved_files->GetAllFileEntries();
return GetSavedFileEntries(ExtensionPrefs::Get(profile_), extension_id);
}

bool SavedFilesService::IsRegistered(const std::string& extension_id,
Expand All @@ -258,19 +261,32 @@ void SavedFilesService::ClearQueueIfNoRetainPermission(
const Extension* extension) {
if (!extension->GetActivePermissions()->HasAPIPermission(
APIPermission::kFileSystemRetainFiles)) {
ClearSavedFileEntries(ExtensionPrefs::Get(profile_), extension->id());
Clear(extension->id());
ClearQueue(extension);
}
}

SavedFilesService::SavedFiles* SavedFilesService::GetOrInsert(
const std::string& extension_id) {
std::map<std::string, SavedFiles*>::iterator it =
void SavedFilesService::ClearQueue(const extensions::Extension* extension) {
ClearSavedFileEntries(ExtensionPrefs::Get(profile_), extension->id());
Clear(extension->id());
}

SavedFilesService::SavedFiles* SavedFilesService::Get(
const std::string& extension_id) const {
std::map<std::string, SavedFiles*>::const_iterator it =
extension_id_to_saved_files_.find(extension_id);
if (it != extension_id_to_saved_files_.end())
return it->second;

SavedFiles* saved_files = new SavedFiles(profile_, extension_id);
return NULL;
}

SavedFilesService::SavedFiles* SavedFilesService::GetOrInsert(
const std::string& extension_id) {
SavedFiles* saved_files = Get(extension_id);
if (saved_files)
return saved_files;

saved_files = new SavedFiles(profile_, extension_id);
extension_id_to_saved_files_.insert(
std::make_pair(extension_id, saved_files));
return saved_files;
Expand Down
6 changes: 6 additions & 0 deletions apps/saved_files_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ class SavedFilesService : public BrowserContextKeyedService,
// fileSystem.retainFiles permission.
void ClearQueueIfNoRetainPermission(const extensions::Extension* extension);

// Clears all retained files.
void ClearQueue(const extensions::Extension* extension);

private:
FRIEND_TEST_ALL_PREFIXES(::SavedFilesServiceUnitTest, RetainTwoFilesTest);
FRIEND_TEST_ALL_PREFIXES(::SavedFilesServiceUnitTest, EvictionTest);
Expand All @@ -110,6 +113,9 @@ class SavedFilesService : public BrowserContextKeyedService,
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE;

// Returns the SavedFiles for |extension_id| or NULL if one does not exist.
SavedFiles* Get(const std::string& extension_id) const;

// Returns the SavedFiles for |extension_id|, creating it if necessary.
SavedFiles* GetOrInsert(const std::string& extension_id);

Expand Down
6 changes: 6 additions & 0 deletions chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -5020,6 +5020,9 @@ Make sure you do not expose any sensitive information.
<message name="IDS_EXTENSION_PROMPT_OAUTH_PERMISSIONS_HEADER" desc="A line of explanatory text that precedes all GAIA account permissions that an extension might request. This is shown when an app requests permissions which are optional.">
On your <ph name="ACCOUNT_EMAIL">$1<ex>user@example.com</ex></ph> account, it could:
</message>
<message name="IDS_EXTENSION_PROMPT_RETAINED_FILES" desc="A line of explanatory text that precedes the list of files the app has permanent access to. This is shown when an app has persistent access to files.">
It has permanent access to:
</message>

<if expr="is_android">
<message name="IDS_EXTENSION_PROMPT_WARNING_FULL_ACCESS" desc="Mobile: Permission string for full access to the device and all websites.">
Expand Down Expand Up @@ -5657,6 +5660,9 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_EXTENSION_PROMPT_PERMISSIONS_ABORT_BUTTON" desc="Text for the deny button on the extension permissions prompt">
Deny
</message>
<message name="IDS_EXTENSION_PROMPT_PERMISSIONS_CLEAR_RETAINED_FILES_BUTTON" desc="Text for the Revoke File Access button on the extension permissions prompt">
Revoke File Access
</message>
<message name="IDS_EXTENSION_WEB_STORE_TITLE" desc="Text for the Chrome Web Store">
Chrome Web Store
</message>
Expand Down
2 changes: 1 addition & 1 deletion chrome/app/nibs/ExtensionInstallPrompt.xib
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@
<reference key="NSSuperview" ref="359431345"/>
<reference key="NSWindow"/>
<reference key="NSNextKeyView" ref="544761792"/>
<int key="NSsFlags">133760</int>
<int key="NSsFlags">133776</int>
<reference key="NSVScroller" ref="636351768"/>
<reference key="NSHScroller" ref="858919758"/>
<reference key="NSContentView" ref="544761792"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include "chrome/browser/extensions/api/developer_private/developer_private_api.h"

#include "apps/app_load_service.h"
#include "apps/app_restore_service.h"
#include "apps/saved_files_service.h"
#include "base/base64.h"
#include "base/command_line.h"
#include "base/file_util.h"
Expand Down Expand Up @@ -508,31 +510,44 @@ bool DeveloperPrivateReloadFunction::RunImpl() {
}

bool DeveloperPrivateShowPermissionsDialogFunction::RunImpl() {
std::string extension_id;
EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &extension_id));
EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &extension_id_));
ExtensionService* service = profile()->GetExtensionService();
CHECK(!extension_id.empty());
CHECK(!extension_id_.empty());
ShellWindowRegistry* registry = ShellWindowRegistry::Get(profile());
DCHECK(registry);
ShellWindow* shell_window = registry->GetShellWindowForRenderViewHost(
render_view_host());
prompt_.reset(new ExtensionInstallPrompt(shell_window->web_contents()));
const Extension* extension = service->GetInstalledExtension(extension_id);
const Extension* extension = service->GetInstalledExtension(extension_id_);

if (!extension)
return false;

// Released by InstallUIAbort.
// Released by InstallUIAbort or InstallUIProceed.
AddRef();
prompt_->ReviewPermissions(this, extension);
std::vector<base::FilePath> retained_file_paths;
if (extension->HasAPIPermission(extensions::APIPermission::kFileSystem)) {
std::vector<apps::SavedFileEntry> retained_file_entries =
apps::SavedFilesService::Get(profile())->GetAllFileEntries(
extension_id_);
for (size_t i = 0; i < retained_file_entries.size(); i++) {
retained_file_paths.push_back(retained_file_entries[i].path);
}
}
prompt_->ReviewPermissions(this, extension, retained_file_paths);
return true;
}

DeveloperPrivateReloadFunction::~DeveloperPrivateReloadFunction() {}

// This is called when the user clicks "Revoke File Access."
void DeveloperPrivateShowPermissionsDialogFunction::InstallUIProceed() {
// The permissions dialog only contains a close button.
NOTREACHED();
apps::SavedFilesService::Get(profile())->ClearQueue(
profile()->GetExtensionService()->GetExtensionById(extension_id_, true));
if (apps::AppRestoreService::Get(profile())->IsAppRestorable(extension_id_))
apps::AppLoadService::Get(profile())->RestartApplication(extension_id_);
SendResponse(true);
Release();
}

void DeveloperPrivateShowPermissionsDialogFunction::InstallUIAbort(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ class DeveloperPrivateShowPermissionsDialogFunction
virtual void InstallUIAbort(bool user_initiated) OVERRIDE;

scoped_ptr<ExtensionInstallPrompt> prompt_;
std::string extension_id_;

};

Expand Down
39 changes: 35 additions & 4 deletions chrome/browser/extensions/extension_install_prompt.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ static const int kAcceptButtonIds[ExtensionInstallPrompt::NUM_PROMPT_TYPES] = {
IDS_EXTENSION_PROMPT_RE_ENABLE_BUTTON,
IDS_EXTENSION_PROMPT_PERMISSIONS_BUTTON,
0, // External installs use different strings for extensions/apps.
0,
IDS_EXTENSION_PROMPT_PERMISSIONS_CLEAR_RETAINED_FILES_BUTTON,
};
static const int kAbortButtonIds[ExtensionInstallPrompt::NUM_PROMPT_TYPES] = {
0, // These all use the platform's default cancel label.
Expand Down Expand Up @@ -266,11 +266,22 @@ string16 ExtensionInstallPrompt::Prompt::GetHeading() const {
}

int ExtensionInstallPrompt::Prompt::GetDialogButtons() const {
if (type_ == POST_INSTALL_PERMISSIONS_PROMPT &&
ShouldDisplayRevokeFilesButton()) {
return kButtons[type_] | ui::DIALOG_BUTTON_OK;
}

return kButtons[type_];
}

bool ExtensionInstallPrompt::Prompt::HasAcceptButtonLabel() const {
return kAcceptButtonIds[type_] > 0;
if (kAcceptButtonIds[type_] == 0)
return false;

if (type_ == POST_INSTALL_PERMISSIONS_PROMPT)
return ShouldDisplayRevokeFilesButton();

return true;
}

string16 ExtensionInstallPrompt::Prompt::GetAcceptButtonLabel() const {
Expand Down Expand Up @@ -304,6 +315,10 @@ string16 ExtensionInstallPrompt::Prompt::GetOAuthHeading() const {
return l10n_util::GetStringFUTF16(kOAuthHeaderIds[type_], oauth_user_name_);
}

string16 ExtensionInstallPrompt::Prompt::GetRetainedFilesHeading() const {
return l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_RETAINED_FILES);
}

void ExtensionInstallPrompt::Prompt::AppendRatingStars(
StarAppender appender, void* data) const {
CHECK(appender);
Expand Down Expand Up @@ -366,6 +381,19 @@ const IssueAdviceInfoEntry& ExtensionInstallPrompt::Prompt::GetOAuthIssue(
return oauth_issue_advice_[index];
}

size_t ExtensionInstallPrompt::Prompt::GetRetainedFileCount() const {
return retained_files_.size();
}

string16 ExtensionInstallPrompt::Prompt::GetRetainedFile(size_t index) const {
CHECK_LT(index, retained_files_.size());
return base::UTF8ToUTF16(retained_files_[index].AsUTF8Unsafe());
}

bool ExtensionInstallPrompt::Prompt::ShouldDisplayRevokeFilesButton() const {
return !retained_files_.empty();
}

ExtensionInstallPrompt::ShowParams::ShowParams(content::WebContents* contents)
: parent_web_contents(contents),
parent_window(NativeWindowForWebContents(contents)),
Expand Down Expand Up @@ -561,11 +589,14 @@ void ExtensionInstallPrompt::ConfirmIssueAdvice(
LoadImageIfNeeded();
}

void ExtensionInstallPrompt::ReviewPermissions(Delegate* delegate,
const Extension* extension) {
void ExtensionInstallPrompt::ReviewPermissions(
Delegate* delegate,
const Extension* extension,
const std::vector<base::FilePath>& retained_file_paths) {
DCHECK(ui_loop_ == base::MessageLoop::current());
extension_ = extension;
permissions_ = extension->GetActivePermissions();
prompt_.set_retained_files(retained_file_paths);
delegate_ = delegate;
prompt_.set_type(POST_INSTALL_PERMISSIONS_PROMPT);

Expand Down
21 changes: 18 additions & 3 deletions chrome/browser/extensions/extension_install_prompt.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "base/callback.h"
#include "base/compiler_specific.h"
#include "base/files/file_path.h"
#include "base/memory/scoped_ptr.h"
#include "base/string16.h"
#include "chrome/browser/extensions/crx_installer_error.h"
Expand Down Expand Up @@ -89,6 +90,7 @@ class ExtensionInstallPrompt
string16 GetAbortButtonLabel() const;
string16 GetPermissionsHeading() const;
string16 GetOAuthHeading() const;
string16 GetRetainedFilesHeading() const;

// Getters for webstore metadata. Only populated when the type is
// INLINE_INSTALL_PROMPT.
Expand All @@ -105,6 +107,8 @@ class ExtensionInstallPrompt
string16 GetPermission(size_t index) const;
size_t GetOAuthIssueCount() const;
const IssueAdviceInfoEntry& GetOAuthIssue(size_t index) const;
size_t GetRetainedFileCount() const;
string16 GetRetainedFile(size_t index) const;

// Populated for BUNDLE_INSTALL_PROMPT.
const extensions::BundleInstaller* bundle() const { return bundle_; }
Expand All @@ -118,10 +122,17 @@ class ExtensionInstallPrompt
extension_ = extension;
}

// May be populated for POST_INSTALL_PERMISSIONS_PROMPT.
void set_retained_files(const std::vector<base::FilePath>& retained_files) {
retained_files_ = retained_files;
}

const gfx::Image& icon() const { return icon_; }
void set_icon(const gfx::Image& icon) { icon_ = icon; }

private:
bool ShouldDisplayRevokeFilesButton() const;

PromptType type_;

// Permissions that are being requested (may not be all of an extension's
Expand Down Expand Up @@ -149,6 +160,8 @@ class ExtensionInstallPrompt
// Range is kMinExtensionRating to kMaxExtensionRating
double average_rating_;
int rating_count_;

std::vector<base::FilePath> retained_files_;
};

static const int kMinExtensionRating = 0;
Expand Down Expand Up @@ -292,9 +305,11 @@ class ExtensionInstallPrompt
// This is called by the app handler launcher to review what permissions the
// extension or app currently has.
//
// This *WILL* call Abort() on |delegate|.
virtual void ReviewPermissions(Delegate* delegate,
const extensions::Extension* extension);
// We *MUST* eventually call either Proceed() or Abort() on |delegate|.
virtual void ReviewPermissions(
Delegate* delegate,
const extensions::Extension* extension,
const std::vector<base::FilePath>& retained_file_paths);

// Installation was successful. This is declared virtual for testing.
virtual void OnInstallSuccess(const extensions::Extension* extension,
Expand Down
5 changes: 4 additions & 1 deletion chrome/browser/extensions/extension_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ namespace {

// Additional preferences keys

// Whether this extension was running when chrome last shutdown.
// True if this extension is running. Note this preference stops getting updated
// during Chrome shutdown (and won't be updated on a browser crash) and so can
// be used at startup to determine whether the extension was running when Chrome
// was last terminated.
const char kPrefRunning[] = "running";

// Whether this extension had windows when it was last running.
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/extensions/shell_window_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ ShellWindowRegistry::ShellWindowList ShellWindowRegistry::GetShellWindowsForApp(
ShellWindowList app_windows;
for (ShellWindowList::const_iterator i = shell_windows_.begin();
i != shell_windows_.end(); ++i) {
if ((*i)->extension()->id() == app_id)
if ((*i)->extension_id() == app_id)
app_windows.push_back(*i);
}
return app_windows;
Expand Down
Loading

0 comments on commit a2886e8

Please sign in to comment.