Hello andreip,
I'd like you to do a code review. Please execute
g4 diff -c 9447213
or point your web browser to
http://mondrian/9447213
to review the following code:
Change 9447213 by stevebl...@steveblock-gears3 on 2008/12/17 12:21:29 *pending*
Adds BrowsingContext parameter to
PermissionsManager::AcquirePermission. Mailed on bahalf of Opera.
R=andreip
[email protected],[email protected]
DELTA=16 (10 added, 0 deleted, 6 changed)
OCL=9447213
Affected files ...
...
//depot/googleclient/gears/opensource/gears/base/common/permissions_manager.cc#3
edit
...
//depot/googleclient/gears/opensource/gears/base/common/permissions_manager.h#2
edit
... //depot/googleclient/gears/opensource/gears/factory/factory_impl.cc#21 edit
... //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#69
edit
16 delta lines: 10 added, 0 deleted, 6 changed
Also consider running:
g4 lint -c 9447213
which verifies that the changelist doesn't introduce new style violations.
If you can't do the review, please let me know as soon as possible. During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately. Visit
http://www/eng/code_review.html for more information.
This is a semiautomated message from "g4 mail". Complaints or suggestions?
Mail [email protected].
Change 9447213 by stevebl...@steveblock-gears3 on 2008/12/17 12:21:29 *pending*
Adds BrowsingContext parameter to
PermissionsManager::AcquirePermission. Mailed on bahalf of Opera.
R=andreip
[email protected],[email protected]
DELTA=17 (9 added, 0 deleted, 8 changed)
OCL=9447213
Affected files ...
...
//depot/googleclient/gears/opensource/gears/base/common/permissions_manager.cc#3
edit
...
//depot/googleclient/gears/opensource/gears/base/common/permissions_manager.h#2
edit
... //depot/googleclient/gears/opensource/gears/factory/factory_impl.cc#21 edit
... //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#69
edit
====
//depot/googleclient/gears/opensource/gears/base/common/permissions_manager.cc#3
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/base/common/permissions_manager.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/permissions_manager.cc
2008-12-17 13:38:25.000000000 +0000
+++ googleclient/gears/opensource/gears/base/common/permissions_manager.cc
2008-12-17 13:51:21.000000000 +0000
@@ -37,12 +37,14 @@
: security_origin_(security_origin), is_worker_(is_worker) {
}
-bool PermissionsManager::AcquirePermission(PermissionsDB::PermissionType type)
{
- return AcquirePermission(type, NULL);
+bool PermissionsManager::AcquirePermission(PermissionsDB::PermissionType type,
+ BrowsingContext *context) {
+ return AcquirePermission(type, context, NULL);
}
bool PermissionsManager::AcquirePermission(
PermissionsDB::PermissionType type,
+ BrowsingContext *context,
const PermissionsDialog::CustomContent *custom) {
// Check if we already have a decision.
@@ -51,7 +53,7 @@
permission_state_[type] = PermissionsDialog::Prompt(security_origin_,
type,
custom,
- NULL);
+ context);
}
// Return the boolean decision.
====
//depot/googleclient/gears/opensource/gears/base/common/permissions_manager.h#2
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/base/common/permissions_manager.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/permissions_manager.h
2008-11-10 14:29:57.000000000 +0000
+++ googleclient/gears/opensource/gears/base/common/permissions_manager.h
2008-12-17 13:51:35.000000000 +0000
@@ -31,19 +31,23 @@
#include "gears/base/common/permissions_db.h"
#include "gears/ui/common/permissions_dialog.h"
+class BrowsingContext;
+
class PermissionsManager {
public:
PermissionsManager(const SecurityOrigin &security_origin, bool is_worker);
// Attempts to acquire the given type of permission. If the permission is not
// currently set (either temporarily or in the database), it prompts the
user.
- bool AcquirePermission(PermissionsDB::PermissionType type);
+ bool AcquirePermission(PermissionsDB::PermissionType type,
+ BrowsingContext *context);
// Attempts to acquire the given type of permission. If the permission is not
// currently set (either temporarily or in the database), it prompts the
user.
// The permissions prompt is customized with the contents of the 'custom'
// object.
bool AcquirePermission(PermissionsDB::PermissionType type,
+ BrowsingContext *context,
const PermissionsDialog::CustomContent *custom);
// Returns true if the owning module has the given permission type and false
==== //depot/googleclient/gears/opensource/gears/factory/factory_impl.cc#21 -
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/factory/factory_impl.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/factory/factory_impl.cc 2008-12-17
12:17:37.000000000 +0000
+++ googleclient/gears/opensource/gears/factory/factory_impl.cc 2008-12-17
13:49:08.000000000 +0000
@@ -114,7 +114,7 @@
if (!EnvIsWorker()) {
MaybeNotifyUserOfVersionCollision(); // only notifies once per process
}
-
+
const char16 *error_text = GetVersionCollisionErrorString();
if (error_text) {
context->SetException(error_text);
@@ -154,7 +154,8 @@
// Make sure the user gives this site permission to use Gears unless the
// module can be created without requiring any permissions.
if (!GetPermissionsManager()->AcquirePermission(
- PermissionsDB::PERMISSION_LOCAL_DATA)) {
+ PermissionsDB::PERMISSION_LOCAL_DATA,
+ EnvPageBrowsingContext())) {
context->SetException(kPermissionExceptionString);
return;
}
@@ -171,7 +172,7 @@
// Do case-sensitive comparisons, which are always better in APIs. They make
// code consistent across callers, and they are easier to support over time.
scoped_refptr<ModuleImplBaseClass> object;
-
+
if (module_name == STRING16(L"beta.database")) {
CreateModule<GearsDatabase>(module_environment_.get(), context, &object);
} else if (module_name == STRING16(L"beta.desktop")) {
@@ -258,11 +259,12 @@
void GearsFactoryImpl::GetPermission(JsCallContext *context) {
scoped_ptr<PermissionsDialog::CustomContent> custom_content(
PermissionsDialog::CreateCustomContent(context));
-
+
if (!custom_content.get()) { return; }
-
+
bool has_permission = GetPermissionsManager()->AcquirePermission(
PermissionsDB::PERMISSION_LOCAL_DATA,
+ EnvPageBrowsingContext(),
custom_content.get());
context->SetReturnValue(JSPARAM_BOOL, &has_permission);
==== //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#69
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/geolocation/geolocation.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/geolocation.cc
2008-12-17 13:38:25.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/geolocation.cc
2008-12-17 13:48:24.000000000 +0000
@@ -381,6 +381,7 @@
bool has_permission = GetPermissionsManager()->AcquirePermission(
PermissionsDB::PERMISSION_LOCATION_DATA,
+ EnvPageBrowsingContext(),
custom_content.get());
context->SetReturnValue(JSPARAM_BOOL, &has_permission);
@@ -1626,7 +1627,8 @@
static bool AcquirePermissionForLocationData(ModuleImplBaseClass *geo_module,
JsCallContext *context) {
if (!geo_module->GetPermissionsManager()->AcquirePermission(
- PermissionsDB::PERMISSION_LOCATION_DATA)) {
+ PermissionsDB::PERMISSION_LOCATION_DATA,
+ geo_module->EnvPageBrowsingContext())) {
std::string16 error = STRING16(L"Page does not have permission to access "
L"location information using "
PRODUCT_FRIENDLY_NAME);
