Commit ad93c6ba authored by asargent@chromium.org's avatar asargent@chromium.org

Change the web store private install API to accept a localized extension name.

In our initial design of the beginInstallWithManifest function, we forgot that
some extensions have a name which needs token substitution for i18n/l10n. This
change renames beginInstallWithManifest to beginInstallWithManifest2, and
modifies the signature to take optional localizedName and locale parameters.

I also refactored how we compare the passed in manifest used in the
pre-download confirmation dialog - we now keep a copy of the source from the
.crx file to compare against, since i18n substututions can happen on the
manifest we use to construct the Extension object.

BUG=75821
TEST=Covered by browser tests for now; will require web store server-side
changes before it can be manually tested.

Review URL: http://codereview.chromium.org/6992047

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@86780 0039d316-1c4b-4281-b951-d872f2087c98
parent b5cca98e
...@@ -41,63 +41,65 @@ ...@@ -41,63 +41,65 @@
namespace { namespace {
struct WhitelistedInstallData { struct Whitelist {
WhitelistedInstallData() {} Whitelist() {}
std::set<std::string> ids; std::set<std::string> ids;
std::map<std::string, linked_ptr<DictionaryValue> > manifests; std::map<std::string, linked_ptr<CrxInstaller::WhitelistEntry> > entries;
}; };
static base::LazyInstance<WhitelistedInstallData> static base::LazyInstance<Whitelist>
g_whitelisted_install_data(base::LINKER_INITIALIZED); g_whitelisted_install_data(base::LINKER_INITIALIZED);
} // namespace } // namespace
// static // static
void CrxInstaller::SetWhitelistedInstallId(const std::string& id) { void CrxInstaller::SetWhitelistedInstallId(const std::string& id) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
g_whitelisted_install_data.Get().ids.insert(id); g_whitelisted_install_data.Get().ids.insert(id);
} }
// static // static
void CrxInstaller::SetWhitelistedManifest(const std::string& id, void CrxInstaller::SetWhitelistEntry(const std::string& id,
DictionaryValue* parsed_manifest) { CrxInstaller::WhitelistEntry* entry) {
CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
WhitelistedInstallData& data = g_whitelisted_install_data.Get(); Whitelist& data = g_whitelisted_install_data.Get();
data.manifests[id] = linked_ptr<DictionaryValue>(parsed_manifest); data.entries[id] = linked_ptr<CrxInstaller::WhitelistEntry>(entry);
} }
// static // static
const DictionaryValue* CrxInstaller::GetWhitelistedManifest( const CrxInstaller::WhitelistEntry* CrxInstaller::GetWhitelistEntry(
const std::string& id) { const std::string& id) {
WhitelistedInstallData& data = g_whitelisted_install_data.Get(); CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (ContainsKey(data.manifests, id)) Whitelist& data = g_whitelisted_install_data.Get();
return data.manifests[id].get(); if (ContainsKey(data.entries, id))
return data.entries[id].get();
else else
return NULL; return NULL;
} }
// static // static
DictionaryValue* CrxInstaller::RemoveWhitelistedManifest( CrxInstaller::WhitelistEntry* CrxInstaller::RemoveWhitelistEntry(
const std::string& id) { const std::string& id) {
WhitelistedInstallData& data = g_whitelisted_install_data.Get(); CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (ContainsKey(data.manifests, id)) { Whitelist& data = g_whitelisted_install_data.Get();
DictionaryValue* manifest = data.manifests[id].release(); if (ContainsKey(data.entries, id)) {
data.manifests.erase(id); CrxInstaller::WhitelistEntry* entry = data.entries[id].release();
return manifest; data.entries.erase(id);
return entry;
} }
return NULL; return NULL;
} }
// static // static
bool CrxInstaller::IsIdWhitelisted(const std::string& id) { bool CrxInstaller::IsIdWhitelisted(const std::string& id) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
std::set<std::string>& ids = g_whitelisted_install_data.Get().ids; std::set<std::string>& ids = g_whitelisted_install_data.Get().ids;
return ContainsKey(ids, id); return ContainsKey(ids, id);
} }
// static // static
bool CrxInstaller::ClearWhitelistedInstallId(const std::string& id) { bool CrxInstaller::ClearWhitelistedInstallId(const std::string& id) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
std::set<std::string>& ids = g_whitelisted_install_data.Get().ids; std::set<std::string>& ids = g_whitelisted_install_data.Get().ids;
if (ContainsKey(ids, id)) { if (ContainsKey(ids, id)) {
ids.erase(id); ids.erase(id);
...@@ -185,7 +187,7 @@ void CrxInstaller::ConvertUserScriptOnFileThread() { ...@@ -185,7 +187,7 @@ void CrxInstaller::ConvertUserScriptOnFileThread() {
return; return;
} }
OnUnpackSuccess(extension->path(), extension->path(), extension); OnUnpackSuccess(extension->path(), extension->path(), NULL, extension);
} }
void CrxInstaller::InstallWebApp(const WebApplicationInfo& web_app) { void CrxInstaller::InstallWebApp(const WebApplicationInfo& web_app) {
...@@ -209,7 +211,7 @@ void CrxInstaller::ConvertWebAppOnFileThread( ...@@ -209,7 +211,7 @@ void CrxInstaller::ConvertWebAppOnFileThread(
// TODO(aa): conversion data gets lost here :( // TODO(aa): conversion data gets lost here :(
OnUnpackSuccess(extension->path(), extension->path(), extension); OnUnpackSuccess(extension->path(), extension->path(), NULL, extension);
} }
bool CrxInstaller::AllowInstall(const Extension* extension, bool CrxInstaller::AllowInstall(const Extension* extension,
...@@ -311,6 +313,7 @@ void CrxInstaller::OnUnpackFailure(const std::string& error_message) { ...@@ -311,6 +313,7 @@ void CrxInstaller::OnUnpackFailure(const std::string& error_message) {
void CrxInstaller::OnUnpackSuccess(const FilePath& temp_dir, void CrxInstaller::OnUnpackSuccess(const FilePath& temp_dir,
const FilePath& extension_dir, const FilePath& extension_dir,
const DictionaryValue* original_manifest,
const Extension* extension) { const Extension* extension) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
...@@ -326,6 +329,9 @@ void CrxInstaller::OnUnpackSuccess(const FilePath& temp_dir, ...@@ -326,6 +329,9 @@ void CrxInstaller::OnUnpackSuccess(const FilePath& temp_dir,
extension_ = extension; extension_ = extension;
temp_dir_ = temp_dir; temp_dir_ = temp_dir;
if (original_manifest)
original_manifest_.reset(original_manifest->DeepCopy());
// We don't have to delete the unpack dir explicity since it is a child of // We don't have to delete the unpack dir explicity since it is a child of
// the temp dir. // the temp dir.
unpacked_extension_root_ = extension_dir; unpacked_extension_root_ = extension_dir;
...@@ -347,17 +353,6 @@ void CrxInstaller::OnUnpackSuccess(const FilePath& temp_dir, ...@@ -347,17 +353,6 @@ void CrxInstaller::OnUnpackSuccess(const FilePath& temp_dir,
NOTREACHED(); NOTREACHED();
} }
// Helper method to let us compare a whitelisted manifest with the actual
// downloaded extension's manifest, but ignoring the kPublicKey since the
// whitelisted manifest doesn't have that value.
static bool EqualsIgnoringPublicKey(
const DictionaryValue& extension_manifest,
const DictionaryValue& whitelisted_manifest) {
scoped_ptr<DictionaryValue> manifest_copy(extension_manifest.DeepCopy());
manifest_copy->Remove(extension_manifest_keys::kPublicKey, NULL);
return manifest_copy->Equals(&whitelisted_manifest);
}
void CrxInstaller::ConfirmInstall() { void CrxInstaller::ConfirmInstall() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (!frontend_weak_.get()) if (!frontend_weak_.get())
...@@ -398,12 +393,11 @@ void CrxInstaller::ConfirmInstall() { ...@@ -398,12 +393,11 @@ void CrxInstaller::ConfirmInstall() {
bool whitelisted = ClearWhitelistedInstallId(extension_->id()) && bool whitelisted = ClearWhitelistedInstallId(extension_->id()) &&
extension_->plugins().empty() && is_gallery_install_; extension_->plugins().empty() && is_gallery_install_;
// Now check if it's whitelisted by manifest. // Now check if there's a WhitelistEntry.
scoped_ptr<DictionaryValue> whitelisted_manifest( scoped_ptr<CrxInstaller::WhitelistEntry> entry(
RemoveWhitelistedManifest(extension_->id())); RemoveWhitelistEntry(extension_->id()));
if (is_gallery_install_ && whitelisted_manifest.get()) { if (is_gallery_install_ && entry.get() && original_manifest_.get()) {
if (!EqualsIgnoringPublicKey(*extension_->manifest_value(), if (!(original_manifest_->Equals(entry->parsed_manifest.get()))) {
*whitelisted_manifest)) {
ReportFailureFromUIThread( ReportFailureFromUIThread(
l10n_util::GetStringUTF8(IDS_EXTENSION_MANIFEST_INVALID)); l10n_util::GetStringUTF8(IDS_EXTENSION_MANIFEST_INVALID));
return; return;
......
...@@ -55,21 +55,25 @@ class CrxInstaller ...@@ -55,21 +55,25 @@ class CrxInstaller
// crbug.com/54916 // crbug.com/54916
static void SetWhitelistedInstallId(const std::string& id); static void SetWhitelistedInstallId(const std::string& id);
struct WhitelistEntry {
scoped_ptr<DictionaryValue> parsed_manifest;
std::string localized_name;
};
// Exempt the next extension install with |id| from displaying a confirmation // Exempt the next extension install with |id| from displaying a confirmation
// prompt, since the user already agreed to the install via // prompt, since the user already agreed to the install via
// beginInstallWithManifest. We require that the extension manifest matches // beginInstallWithManifest. We require that the extension manifest matches
// |parsed_manifest| which is what was used to prompt with. Ownership of // the manifest in |entry|, which is what was used to prompt with. Ownership
// |parsed_manifest| is transferred here. // of |entry| is transferred here.
static void SetWhitelistedManifest(const std::string& id, static void SetWhitelistEntry(const std::string& id, WhitelistEntry* entry);
DictionaryValue* parsed_manifest);
// Returns the previously stored manifest from a call to // Returns the previously stored manifest from a call to SetWhitelistEntry.
// SetWhitelistedManifest. static const CrxInstaller::WhitelistEntry* GetWhitelistEntry(
static const DictionaryValue* GetWhitelistedManifest(const std::string& id); const std::string& id);
// Removes any whitelisted manifest for |id| and returns it. The caller owns // Removes any whitelist data for |id| and returns it. The caller owns
// the return value and is responsible for deleting it. // the return value and is responsible for deleting it.
static DictionaryValue* RemoveWhitelistedManifest(const std::string& id); static WhitelistEntry* RemoveWhitelistEntry(const std::string& id);
// Returns whether |id| is whitelisted - only call this on the UI thread. // Returns whether |id| is whitelisted - only call this on the UI thread.
static bool IsIdWhitelisted(const std::string& id); static bool IsIdWhitelisted(const std::string& id);
...@@ -159,6 +163,7 @@ class CrxInstaller ...@@ -159,6 +163,7 @@ class CrxInstaller
virtual void OnUnpackFailure(const std::string& error_message); virtual void OnUnpackFailure(const std::string& error_message);
virtual void OnUnpackSuccess(const FilePath& temp_dir, virtual void OnUnpackSuccess(const FilePath& temp_dir,
const FilePath& extension_dir, const FilePath& extension_dir,
const DictionaryValue* original_manifest,
const Extension* extension); const Extension* extension);
// Returns true if we can skip confirmation because the install was // Returns true if we can skip confirmation because the install was
...@@ -225,6 +230,10 @@ class CrxInstaller ...@@ -225,6 +230,10 @@ class CrxInstaller
// ExtensionService on success, or delete it on failure. // ExtensionService on success, or delete it on failure.
scoped_refptr<const Extension> extension_; scoped_refptr<const Extension> extension_;
// A parsed copy of the unmodified original manifest, before any
// transformations like localization have taken place.
scoped_ptr<DictionaryValue> original_manifest_;
// If non-empty, contains the current version of the extension we're // If non-empty, contains the current version of the extension we're
// installing (for upgrades). // installing (for upgrades).
std::string current_version_; std::string current_version_;
......
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_switches.h"
#include "chrome/common/extensions/extension_constants.h" #include "chrome/common/extensions/extension_constants.h"
#include "chrome/common/extensions/extension_error_utils.h" #include "chrome/common/extensions/extension_error_utils.h"
#include "chrome/common/extensions/extension_l10n_util.h"
#include "chrome/common/net/gaia/gaia_constants.h" #include "chrome/common/net/gaia/gaia_constants.h"
#include "content/browser/tab_contents/tab_contents.h" #include "content/browser/tab_contents/tab_contents.h"
#include "content/common/notification_details.h" #include "content/common/notification_details.h"
...@@ -33,8 +34,13 @@ ...@@ -33,8 +34,13 @@
namespace { namespace {
const char kIconDataKey[] = "iconData";
const char kIdKey[] = "id";
const char kLocalizedNameKey[] = "localizedName";
const char kLoginKey[] = "login"; const char kLoginKey[] = "login";
const char kManifestKey[] = "manifest";
const char kTokenKey[] = "token"; const char kTokenKey[] = "token";
const char kImageDecodeError[] = "Image decode failed"; const char kImageDecodeError[] = "Image decode failed";
const char kInvalidIdError[] = "Invalid id"; const char kInvalidIdError[] = "Invalid id";
const char kInvalidManifestError[] = "Invalid manifest"; const char kInvalidManifestError[] = "Invalid manifest";
...@@ -283,15 +289,25 @@ bool BeginInstallWithManifestFunction::RunImpl() { ...@@ -283,15 +289,25 @@ bool BeginInstallWithManifestFunction::RunImpl() {
return false; return false;
} }
EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &id_)); DictionaryValue* details = NULL;
EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &details));
CHECK(details);
EXTENSION_FUNCTION_VALIDATE(details->GetString(kIdKey, &id_));
if (!Extension::IdIsValid(id_)) { if (!Extension::IdIsValid(id_)) {
SetResult(INVALID_ID); SetResult(INVALID_ID);
error_ = kInvalidIdError; error_ = kInvalidIdError;
return false; return false;
} }
EXTENSION_FUNCTION_VALIDATE(args_->GetString(1, &icon_data_)); EXTENSION_FUNCTION_VALIDATE(details->GetString(kManifestKey, &manifest_));
EXTENSION_FUNCTION_VALIDATE(args_->GetString(2, &manifest_));
if (details->HasKey(kIconDataKey))
EXTENSION_FUNCTION_VALIDATE(details->GetString(kIconDataKey, &icon_data_));
if (details->HasKey(kLocalizedNameKey))
EXTENSION_FUNCTION_VALIDATE(details->GetString(kLocalizedNameKey,
&localized_name_));
scoped_refptr<SafeBeginInstallHelper> helper = scoped_refptr<SafeBeginInstallHelper> helper =
new SafeBeginInstallHelper(this, icon_data_, manifest_); new SafeBeginInstallHelper(this, icon_data_, manifest_);
...@@ -354,19 +370,29 @@ void BeginInstallWithManifestFunction::OnParseSuccess( ...@@ -354,19 +370,29 @@ void BeginInstallWithManifestFunction::OnParseSuccess(
icon_ = icon; icon_ = icon;
parsed_manifest_.reset(parsed_manifest); parsed_manifest_.reset(parsed_manifest);
// If we were passed a localized name to use in the dialog, create a copy
// of the original manifest and replace the name in it.
scoped_ptr<DictionaryValue> localized_manifest;
if (!localized_name_.empty()) {
localized_manifest.reset(parsed_manifest->DeepCopy());
localized_manifest->SetString(extension_manifest_keys::kName,
localized_name_);
}
// Create a dummy extension and show the extension install confirmation // Create a dummy extension and show the extension install confirmation
// dialog. // dialog.
std::string init_errors; std::string init_errors;
dummy_extension_ = Extension::Create( dummy_extension_ = Extension::Create(
FilePath(), FilePath(),
Extension::INTERNAL, Extension::INTERNAL,
*static_cast<DictionaryValue*>(parsed_manifest_.get()), localized_manifest.get() ? *localized_manifest.get() : *parsed_manifest,
Extension::NO_FLAGS, Extension::NO_FLAGS,
&init_errors); &init_errors);
if (!dummy_extension_.get()) { if (!dummy_extension_.get()) {
OnParseFailure(MANIFEST_ERROR, std::string(kInvalidManifestError)); OnParseFailure(MANIFEST_ERROR, std::string(kInvalidManifestError));
return; return;
} }
if (icon_.empty()) if (icon_.empty())
icon_ = Extension::GetDefaultIcon(dummy_extension_->is_app()); icon_ = Extension::GetDefaultIcon(dummy_extension_->is_app());
...@@ -401,7 +427,10 @@ void BeginInstallWithManifestFunction::OnParseFailure( ...@@ -401,7 +427,10 @@ void BeginInstallWithManifestFunction::OnParseFailure(
} }
void BeginInstallWithManifestFunction::InstallUIProceed() { void BeginInstallWithManifestFunction::InstallUIProceed() {
CrxInstaller::SetWhitelistedManifest(id_, parsed_manifest_.release()); CrxInstaller::WhitelistEntry* entry = new CrxInstaller::WhitelistEntry;
entry->parsed_manifest.reset(parsed_manifest_.release());
entry->localized_name = localized_name_;
CrxInstaller::SetWhitelistEntry(id_, entry);
SetResult(ERROR_NONE); SetResult(ERROR_NONE);
SendResponse(true); SendResponse(true);
...@@ -430,7 +459,7 @@ bool CompleteInstallFunction::RunImpl() { ...@@ -430,7 +459,7 @@ bool CompleteInstallFunction::RunImpl() {
} }
if (!CrxInstaller::IsIdWhitelisted(id) && if (!CrxInstaller::IsIdWhitelisted(id) &&
!CrxInstaller::GetWhitelistedManifest(id)) { !CrxInstaller::GetWhitelistEntry(id)) {
error_ = ExtensionErrorUtils::FormatErrorMessage( error_ = ExtensionErrorUtils::FormatErrorMessage(
kNoPreviousBeginInstallError, id); kNoPreviousBeginInstallError, id);
return false; return false;
......
...@@ -106,6 +106,7 @@ class BeginInstallWithManifestFunction : public AsyncExtensionFunction, ...@@ -106,6 +106,7 @@ class BeginInstallWithManifestFunction : public AsyncExtensionFunction,
std::string id_; std::string id_;
std::string manifest_; std::string manifest_;
std::string icon_data_; std::string icon_data_;
std::string localized_name_;
// The results of parsing manifest_ and icon_data_ go into these two. // The results of parsing manifest_ and icon_data_ go into these two.
scoped_ptr<DictionaryValue> parsed_manifest_; scoped_ptr<DictionaryValue> parsed_manifest_;
...@@ -114,7 +115,7 @@ class BeginInstallWithManifestFunction : public AsyncExtensionFunction, ...@@ -114,7 +115,7 @@ class BeginInstallWithManifestFunction : public AsyncExtensionFunction,
// A dummy Extension object we create for the purposes of using // A dummy Extension object we create for the purposes of using
// ExtensionInstallUI to prompt for confirmation of the install. // ExtensionInstallUI to prompt for confirmation of the install.
scoped_refptr<Extension> dummy_extension_; scoped_refptr<Extension> dummy_extension_;
DECLARE_EXTENSION_FUNCTION_NAME("webstorePrivate.beginInstallWithManifest"); DECLARE_EXTENSION_FUNCTION_NAME("webstorePrivate.beginInstallWithManifest2");
}; };
class CompleteInstallFunction : public SyncExtensionFunction { class CompleteInstallFunction : public SyncExtensionFunction {
......
...@@ -51,9 +51,10 @@ class ExtensionWebstorePrivateApiTest : public ExtensionApiTest { ...@@ -51,9 +51,10 @@ class ExtensionWebstorePrivateApiTest : public ExtensionApiTest {
return url.ReplaceComponents(replace_host); return url.ReplaceComponents(replace_host);
} }
// Navigates to |page| and runs the Extension API test there. // Navigates to |page| and runs the Extension API test there. Any downloads
bool RunInstallTest(const std::string& page) { // of extensions will return the contents of |crx_file|.
GURL crx_url = GetTestServerURL("extension.crx"); bool RunInstallTest(const std::string& page, const std::string& crx_file) {
GURL crx_url = GetTestServerURL(crx_file);
CommandLine::ForCurrentProcess()->AppendSwitchASCII( CommandLine::ForCurrentProcess()->AppendSwitchASCII(
switches::kAppsGalleryUpdateURL, crx_url.spec()); switches::kAppsGalleryUpdateURL, crx_url.spec());
...@@ -69,18 +70,23 @@ class ExtensionWebstorePrivateApiTest : public ExtensionApiTest { ...@@ -69,18 +70,23 @@ class ExtensionWebstorePrivateApiTest : public ExtensionApiTest {
// TODO(asargent) - flaky; see crbug.com/80606. // TODO(asargent) - flaky; see crbug.com/80606.
// Test cases where the user accepts the install confirmation dialog. // Test cases where the user accepts the install confirmation dialog.
IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, FLAKY_InstallAccepted) { IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, FLAKY_InstallAccepted) {
ASSERT_TRUE(RunInstallTest("accepted.html")); ASSERT_TRUE(RunInstallTest("accepted.html", "extension.crx"));
}
// Tests passing a localized name.
IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, InstallLocalized) {
ASSERT_TRUE(RunInstallTest("localized.html", "localized_extension.crx"));
} }
// Now test the case where the user cancels the confirmation dialog. // Now test the case where the user cancels the confirmation dialog.
IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, InstallCancelled) { IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, InstallCancelled) {
BeginInstallWithManifestFunction::SetAutoConfirmForTests(false); BeginInstallWithManifestFunction::SetAutoConfirmForTests(false);
ASSERT_TRUE(RunInstallTest("cancelled.html")); ASSERT_TRUE(RunInstallTest("cancelled.html", "extension.crx"));
} }
IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, InstallNoGesture) { IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, InstallNoGesture) {
BeginInstallFunction::SetIgnoreUserGestureForTests(false); BeginInstallFunction::SetIgnoreUserGestureForTests(false);
ASSERT_TRUE(RunInstallTest("no_user_gesture.html")); ASSERT_TRUE(RunInstallTest("no_user_gesture.html", "extension.crx"));
} }
IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest,
...@@ -88,7 +94,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, ...@@ -88,7 +94,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest,
ui_test_utils::WindowedNotificationObserver observer( ui_test_utils::WindowedNotificationObserver observer(
NotificationType::EXTENSION_INSTALL_ERROR, NotificationType::EXTENSION_INSTALL_ERROR,
NotificationService::AllSources()); NotificationService::AllSources());
ASSERT_TRUE(RunInstallTest("incorrect_manifest1.html")); ASSERT_TRUE(RunInstallTest("incorrect_manifest1.html", "extension.crx"));
observer.Wait(); observer.Wait();
} }
...@@ -97,6 +103,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, ...@@ -97,6 +103,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest,
ui_test_utils::WindowedNotificationObserver observer( ui_test_utils::WindowedNotificationObserver observer(
NotificationType::EXTENSION_INSTALL_ERROR, NotificationType::EXTENSION_INSTALL_ERROR,
NotificationService::AllSources()); NotificationService::AllSources());
ASSERT_TRUE(RunInstallTest("incorrect_manifest2.html")); ASSERT_TRUE(RunInstallTest("incorrect_manifest2.html", "extension.crx"));
observer.Wait(); observer.Wait();
} }
...@@ -284,7 +284,7 @@ void SandboxedExtensionUnpacker::OnUnpackExtensionSucceeded( ...@@ -284,7 +284,7 @@ void SandboxedExtensionUnpacker::OnUnpackExtensionSucceeded(
if (!RewriteCatalogFiles()) if (!RewriteCatalogFiles())
return; return;
ReportSuccess(); ReportSuccess(manifest);
} }
void SandboxedExtensionUnpacker::OnUnpackExtensionFailed( void SandboxedExtensionUnpacker::OnUnpackExtensionFailed(
...@@ -480,14 +480,18 @@ void SandboxedExtensionUnpacker::ReportFailure(FailureReason reason, ...@@ -480,14 +480,18 @@ void SandboxedExtensionUnpacker::ReportFailure(FailureReason reason,
client_->OnUnpackFailure(error); client_->OnUnpackFailure(error);
} }
void SandboxedExtensionUnpacker::ReportSuccess() { void SandboxedExtensionUnpacker::ReportSuccess(
const DictionaryValue& original_manifest) {
UMA_HISTOGRAM_COUNTS("Extensions.SandboxUnpackSuccess", 1); UMA_HISTOGRAM_COUNTS("Extensions.SandboxUnpackSuccess", 1);
RecordSuccessfulUnpackTimeHistograms( RecordSuccessfulUnpackTimeHistograms(
crx_path_, base::TimeTicks::Now() - unpack_start_time_); crx_path_, base::TimeTicks::Now() - unpack_start_time_);
// Client takes ownership of temporary directory and extension. // Client takes ownership of temporary directory and extension.
client_->OnUnpackSuccess(temp_dir_.Take(), extension_root_, extension_); client_->OnUnpackSuccess(temp_dir_.Take(),
extension_root_,
&original_manifest,
extension_);
extension_ = NULL; extension_ = NULL;
} }
......
...@@ -25,10 +25,14 @@ class SandboxedExtensionUnpackerClient ...@@ -25,10 +25,14 @@ class SandboxedExtensionUnpackerClient
// //
// extension_root - The path to the extension root inside of temp_dir. // extension_root - The path to the extension root inside of temp_dir.
// //
// original_manifest - The parsed but unmodified version of the manifest,
// with no modifications such as localization, etc.
//
// extension - The extension that was unpacked. The client is responsible // extension - The extension that was unpacked. The client is responsible
// for deleting this memory. // for deleting this memory.
virtual void OnUnpackSuccess(const FilePath& temp_dir, virtual void OnUnpackSuccess(const FilePath& temp_dir,
const FilePath& extension_root, const FilePath& extension_root,
const DictionaryValue* original_manifest,
const Extension* extension) = 0; const Extension* extension) = 0;
virtual void OnUnpackFailure(const std::string& error) = 0; virtual void OnUnpackFailure(const std::string& error) = 0;
...@@ -189,7 +193,7 @@ class SandboxedExtensionUnpacker : public UtilityProcessHost::Client { ...@@ -189,7 +193,7 @@ class SandboxedExtensionUnpacker : public UtilityProcessHost::Client {
virtual void OnProcessCrashed(int exit_code); virtual void OnProcessCrashed(int exit_code);
void ReportFailure(FailureReason reason, const std::string& message); void ReportFailure(FailureReason reason, const std::string& message);
void ReportSuccess(); void ReportSuccess(const DictionaryValue& original_manifest);
// Overwrites original manifest with safe result from utility process. // Overwrites original manifest with safe result from utility process.
// Returns NULL on error. Caller owns the returned object. // Returns NULL on error. Caller owns the returned object.
......
...@@ -27,6 +27,7 @@ namespace { ...@@ -27,6 +27,7 @@ namespace {
void OnUnpackSuccess(const FilePath& temp_dir, void OnUnpackSuccess(const FilePath& temp_dir,
const FilePath& extension_root, const FilePath& extension_root,
const DictionaryValue* original_manifest,
const Extension* extension) { const Extension* extension) {
// Don't delete temp_dir here, we need to do some post op checking. // Don't delete temp_dir here, we need to do some post op checking.
} }
...@@ -38,16 +39,17 @@ class MockSandboxedExtensionUnpackerClient ...@@ -38,16 +39,17 @@ class MockSandboxedExtensionUnpackerClient
public: public:
virtual ~MockSandboxedExtensionUnpackerClient() {} virtual ~MockSandboxedExtensionUnpackerClient() {}
MOCK_METHOD3(OnUnpackSuccess, MOCK_METHOD4(OnUnpackSuccess,
void(const FilePath& temp_dir, void(const FilePath& temp_dir,
const FilePath& extension_root, const FilePath& extension_root,
const DictionaryValue* original_manifest,
const Extension* extension)); const Extension* extension));
MOCK_METHOD1(OnUnpackFailure, MOCK_METHOD1(OnUnpackFailure,
void(const std::string& error)); void(const std::string& error));
void DelegateToFake() { void DelegateToFake() {
ON_CALL(*this, OnUnpackSuccess(_, _, _)) ON_CALL(*this, OnUnpackSuccess(_, _, _, _))
.WillByDefault(Invoke(::OnUnpackSuccess)); .WillByDefault(Invoke(::OnUnpackSuccess));
} }
}; };
...@@ -157,7 +159,7 @@ class SandboxedExtensionUnpackerTest : public testing::Test { ...@@ -157,7 +159,7 @@ class SandboxedExtensionUnpackerTest : public testing::Test {
}; };
TEST_F(SandboxedExtensionUnpackerTest, NoCatalogsSuccess) { TEST_F(SandboxedExtensionUnpackerTest, NoCatalogsSuccess) {
EXPECT_CALL(*client_, OnUnpackSuccess(_, _, _)); EXPECT_CALL(*client_, OnUnpackSuccess(_, _, _, _));
EXPECT_CALL(*client_, OnUnpackFailure(_)).Times(0); EXPECT_CALL(*client_, OnUnpackFailure(_)).Times(0);
SetupUnpacker("no_l10n.crx"); SetupUnpacker("no_l10n.crx");
...@@ -179,7 +181,7 @@ TEST_F(SandboxedExtensionUnpackerTest, NoCatalogsSuccess) { ...@@ -179,7 +181,7 @@ TEST_F(SandboxedExtensionUnpackerTest, NoCatalogsSuccess) {
} }
TEST_F(SandboxedExtensionUnpackerTest, WithCatalogsSuccess) { TEST_F(SandboxedExtensionUnpackerTest, WithCatalogsSuccess) {
EXPECT_CALL(*client_, OnUnpackSuccess(_, _, _)); EXPECT_CALL(*client_, OnUnpackSuccess(_, _, _, _));
EXPECT_CALL(*client_, OnUnpackFailure(_)).Times(0); EXPECT_CALL(*client_, OnUnpackFailure(_)).Times(0);
SetupUnpacker("good_l10n.crx"); SetupUnpacker("good_l10n.crx");
......
...@@ -5150,26 +5150,40 @@ ...@@ -5150,26 +5150,40 @@
] ]
}, },
{ {
"name": "beginInstallWithManifest", "name": "beginInstallWithManifest2",
"description": "Initiates the install process for the given extension. This must be called during a user gesture.", "description": "Initiates the install process for the given extension. This must be called during a user gesture.",
"parameters": [ "parameters": [
{ {
"name": "id", "name": "details",
"type": "string", "type": "object",
"description": "The id of the extension to be installled.", "properties": {
"minLength": 32, "id": {
"maxLength": 32 "type": "string",
}, "description": "The id of the extension to be installled.",
{ "minLength": 32,
"name": "icon_data", "maxLength": 32
"type": "string", },
"description": "An icon as a base64-encoded image, displayed in a confirmation dialog." "manifest": {
}, "type": "string",
{ "description": "A string with the contents of the extension's manifest.json file. During the install process, the browser will check that the downloaded extension's manifest matches what was passed in here.",
"name": "manifest_json", "minLength": 1
"type": "string", },
"description": "A string with the contents of the extension's manifest.json file. During the install process, the browser will check that the downloaded extension's manifest matches what was passed in here.", "iconData": {
"minLength": 1 "type": "string",
"optional": true,
"description": "An icon as a base64-encoded image, displayed in a confirmation dialog."
},
"localizedName": {
"type": "string",
"optional": true,
"description": "A string to use instead of the raw value of the 'name' key from manifest.json."
},
"locale": {
"type": "string",
"optional": true,
"description": "The name of the locale used for generating localizedName. This should be the name of one of the directories in the _locales folder of the extension, or the default_locale setting from the manifest."
}
}
}, },
{ {
"name": "callback", "name": "callback",
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// Tests where the beginInstallWithManifest dialog would be auto-accepted // Tests where the beginInstallWithManifest2 dialog would be auto-accepted
// (including a few cases where this does not matter). // (including a few cases where this does not matter).
var tests = [ var tests = [
...@@ -17,46 +17,39 @@ var tests = [ ...@@ -17,46 +17,39 @@ var tests = [
function invalidID() { function invalidID() {
var expectedError = "Invalid id"; var expectedError = "Invalid id";
var id = "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz"; var id = "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz";
chrome.webstorePrivate.beginInstallWithManifest( chrome.webstorePrivate.beginInstallWithManifest2(
id, "", getManifest(), callbackFail(expectedError)); { 'id':id, 'manifest':getManifest() }, callbackFail(expectedError));
}, },
function missingVersion() { function missingVersion() {
var manifestObj = JSON.parse(getManifest()); var manifestObj = JSON.parse(getManifest());
delete manifestObj["version"]; delete manifestObj["version"];
var manifest = JSON.stringify(manifestObj); var manifest = JSON.stringify(manifestObj);
chrome.webstorePrivate.beginInstallWithManifest( chrome.webstorePrivate.beginInstallWithManifest2(
extensionId, { 'id':extensionId, 'manifest': manifest },
"",
manifest,
callbackFail("Invalid manifest", function(result) { callbackFail("Invalid manifest", function(result) {
assertEq("manifest_error", result); assertEq("manifest_error", result);
})); }));
}, },
function successfulInstall() { function successfulInstall() {
// See things through all the way to a successful install, and then we // See things through all the way to a successful install.
// uninstall the extension to clean up. listenOnce(chrome.management.onInstalled, callbackPass(function(info) {
listenOnce(chrome.management.onInstalled, function(info) {
assertEq(info.id, extensionId); assertEq(info.id, extensionId);
chrome.management.uninstall(extensionId, callbackPass()); }));
});
var manifest = getManifest(); var manifest = getManifest();
getIconData(function(icon) { getIconData(function(icon) {
// Begin installing. // Begin installing.
chrome.webstorePrivate.beginInstallWithManifest(extensionId, chrome.webstorePrivate.beginInstallWithManifest2(
icon, {'id': extensionId,'iconData': icon, 'manifest': manifest },
manifest, function(result) {
function(result) {
assertNoLastError(); assertNoLastError();
assertEq(result, ""); assertEq(result, "");
// Now complete the installation. // Now complete the installation.
chrome.webstorePrivate.completeInstall(extensionId, function() { chrome.webstorePrivate.completeInstall(extensionId, callbackPass());
assertNoLastError();
});
}); });
}); });
} }
......
<script src="common.js"></script> <script src="common.js"></script>
<script> <script>
// This tests the user cancelling an install started by // This tests the user cancelling an install started by
// beginInstallWithManifest. // beginInstallWithManifest2.
var manifest = getManifest(); var manifest = getManifest();
getIconData(function(icon) { getIconData(function(icon) {
// Begin installing. // Begin installing.
chrome.webstorePrivate.beginInstallWithManifest(extensionId, chrome.webstorePrivate.beginInstallWithManifest2({ 'id':extensionId,
icon, 'iconData': icon,
manifest, 'manifest': manifest },
function(result) { function(result) {
assertEq(result, "user_cancelled"); assertEq(result, "user_cancelled");
succeed(); succeed();
......
...@@ -49,13 +49,15 @@ function getIconData(callback) { ...@@ -49,13 +49,15 @@ function getIconData(callback) {
var cachedManifest = null; var cachedManifest = null;
// This returns the string contents of the extension's manifest file. // This returns the string contents of the extension's manifest file.
function getManifest() { function getManifest(alternativePath) {
if (cachedManifest) if (cachedManifest)
return cachedManifest; return cachedManifest;
// Do a synchronous XHR to get the manifest. // Do a synchronous XHR to get the manifest.
var xhr = new XMLHttpRequest(); var xhr = new XMLHttpRequest();
xhr.open("GET", "extension/manifest.json", false); xhr.open("GET",
alternativePath ? alternativePath : "extension/manifest.json",
false);
xhr.send(null); xhr.send(null);
return xhr.responseText; return xhr.responseText;
} }
...@@ -11,10 +11,8 @@ var manifest = alterManifest(manifestObj); ...@@ -11,10 +11,8 @@ var manifest = alterManifest(manifestObj);
// that we get an install error after unpacking the crx file because the crx // that we get an install error after unpacking the crx file because the crx
// file's manifest won't match what we provided for the confirmation dialog // file's manifest won't match what we provided for the confirmation dialog
// here. // here.
chrome.webstorePrivate.beginInstallWithManifest( chrome.webstorePrivate.beginInstallWithManifest2(
extensionId, { 'id': extensionId, 'manifest': manifest },
"",
manifest,
function(result) { function(result) {
assertNoLastError(); assertNoLastError();
assertEq("", result); assertEq("", result);
......
...@@ -2,8 +2,8 @@ ...@@ -2,8 +2,8 @@
<script> <script>
// We expect that this test is called outside of a user gesture, which should // We expect that this test is called outside of a user gesture, which should
// generate an error. // generate an error.
chrome.webstorePrivate.beginInstallWithManifest( chrome.webstorePrivate.beginInstallWithManifest2(
extensionId, "", getManifest(), function() { { 'id':extensionId, 'manifest': getManifest() }, function() {
assertEq(chrome.extension.lastError.message, assertEq(chrome.extension.lastError.message,
"This function must be called during a user gesture"); "This function must be called during a user gesture");
succeed(); succeed();
......
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