Hello cprince, aa,
I'd like you to do a code review. Please execute
g4 diff -c 8165297
or point your web browser to
http://mondrian/8165297
to review the following code:
Change 8165297 by [EMAIL PROTECTED] on 2008/09/03 16:19:32 *pending*
Updates callsites to use new version of JsCallContext::GetArguments().
This CL should introduce no change in functionality.
This change updates all Gears modules to use the new version of
JsCallContext::GetArguments(), which allows null or undefined
to be passed for optional arguments, so that subsequent arguments can
be specified when an argument is missing. Also, optional
and required arguments can now be present in any order.
As the title says, this CL should not change the behaviour of any Gears
modules. It simply puts in place the functionality
to allow future changes. (It also removes the need for a separate code
path for Geolocation, which currently needs this
functionality.)
It's probably worth module owners checking their code to make sure I
haven't broken anything.
R=cprince,aa
[EMAIL PROTECTED]
DELTA=158 (47 added, 53 deleted, 58 changed)
OCL=8165297
Affected files ...
... //depot/googleclient/gears/opensource/gears/base/common/js_types.cc#45 edit
... //depot/googleclient/gears/opensource/gears/base/common/js_types.h#31 edit
... //depot/googleclient/gears/opensource/gears/blob/blob.cc#10 edit
... //depot/googleclient/gears/opensource/gears/canvas/canvas.cc#16 edit
...
//depot/googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.cc#13
edit
... //depot/googleclient/gears/opensource/gears/cctests/test.cc#78 edit
... //depot/googleclient/gears/opensource/gears/console/console.cc#3 edit
... //depot/googleclient/gears/opensource/gears/database/database.cc#5 edit
... //depot/googleclient/gears/opensource/gears/database2/transaction.cc#6 edit
... //depot/googleclient/gears/opensource/gears/desktop/desktop.cc#47 edit
... //depot/googleclient/gears/opensource/gears/dummy/dummy_module.cc#2 edit
... //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#46
edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/database2_tests.js#4
edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/internal_tests.js#33
edit
...
//depot/googleclient/gears/opensource/gears/ui/common/permissions_dialog.cc#6
edit
158 delta lines: 47 added, 53 deleted, 58 changed
Also consider running:
g4 lint -c 8165297
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 8165297 by [EMAIL PROTECTED] on 2008/09/03 16:19:32 *pending*
Updates callsites to use new version of JsCallContext::GetArguments().
This CL should introduce no change in functionality.
Affected files ...
... //depot/googleclient/gears/opensource/gears/base/common/js_types.cc#45 edit
... //depot/googleclient/gears/opensource/gears/base/common/js_types.h#31 edit
... //depot/googleclient/gears/opensource/gears/blob/blob.cc#10 edit
... //depot/googleclient/gears/opensource/gears/canvas/canvas.cc#16 edit
...
//depot/googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.cc#13
edit
... //depot/googleclient/gears/opensource/gears/cctests/test.cc#78 edit
... //depot/googleclient/gears/opensource/gears/console/console.cc#3 edit
... //depot/googleclient/gears/opensource/gears/database/database.cc#5 edit
... //depot/googleclient/gears/opensource/gears/database2/transaction.cc#6 edit
... //depot/googleclient/gears/opensource/gears/desktop/desktop.cc#47 edit
... //depot/googleclient/gears/opensource/gears/dummy/dummy_module.cc#2 edit
... //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#46
edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/database2_tests.js#4
edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/internal_tests.js#33
edit
...
//depot/googleclient/gears/opensource/gears/ui/common/permissions_dialog.cc#6
edit
==== //depot/googleclient/gears/opensource/gears/base/common/js_types.cc#45 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/base/common/js_types.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/js_types.cc 2008-09-04
12:40:23.000000000 +0100
+++ googleclient/gears/opensource/gears/base/common/js_types.cc 2008-09-04
12:09:40.000000000 +0100
@@ -2238,54 +2238,7 @@
#endif
-int JsCallContext::GetArguments(int output_argc, JsArgument *output_argv) {
- bool has_optional = false;
-
- if (GetArgumentCount() > output_argc) {
- SetException(STRING16(L"Too many parameters."));
- return 0;
- }
-
- for (int i = 0; i < output_argc; ++i) {
- assert(output_argv[i].value_ptr);
-
- has_optional |= output_argv[i].requirement == JSPARAM_OPTIONAL;
- if (output_argv[i].requirement == JSPARAM_REQUIRED)
- assert(!has_optional); // should not have required arg after optional
-
- // TODO(mpcomplete): We need to handle this more generally. We should
- // continue processing arguments for the case where a developer does
- // something like 'method(null, foo)' if the first argument is optional.
- if (i >= GetArgumentCount() ||
- GetArgumentType(i) == JSPARAM_NULL ||
- GetArgumentType(i) == JSPARAM_UNDEFINED) {
- // Out of arguments
- if (output_argv[i].requirement == JSPARAM_REQUIRED) {
- std::string16 msg;
- msg += STRING16(L"Required argument ");
- msg += IntegerToString16(i + 1);
- msg += STRING16(L" is missing.");
- SetException(msg.c_str());
- }
-
- // If failed on index [N], then N args succeeded
- return i;
- }
-
- if (!ConvertTokenToArgument(this, GetArgument(i), &output_argv[i])) {
- std::string16 msg(STRING16(L"Argument "));
- msg += IntegerToString16(i + 1);
- msg += STRING16(L" has invalid type or is outside allowed range.");
- SetException(msg);
- return i;
- }
- }
-
- return output_argc;
-}
-
-bool JsCallContext::GetArguments2(int output_argc, JsArgument *output_argv) {
-
+bool JsCallContext::GetArguments(int output_argc, JsArgument *output_argv) {
if (GetArgumentCount() > output_argc) {
SetException(STRING16(L"Too many parameters."));
return false;
==== //depot/googleclient/gears/opensource/gears/base/common/js_types.h#31 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/base/common/js_types.h ====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/js_types.h 2008-09-04
12:40:23.000000000 +0100
+++ googleclient/gears/opensource/gears/base/common/js_types.h 2008-09-04
12:07:40.000000000 +0100
@@ -528,16 +528,13 @@
~JsCallContext();
// Get the arguments a JavaScript caller has passed into a scriptable method
- // of a native object. Returns the number of arguments successfully read
- // (will bail at the first invalid argument).
- int GetArguments(int argc, JsArgument *argv);
-
- // As above, except that this function tries to extract all given parameters
- // and accepts null for optional arguments. Returns false if an exception
- // occurs and true otherwise.
- // TODO(andreip): post 0.4, replace the above JsCallContext::GetArguments
- // with this implementation and fix all the call sites.
- bool GetArguments2(int argc, JsArgument *argv);
+ // of a native object. Arguments may be required or optional and these types
+ // may be interspersed freely. null or undefined may be passed for optional
+ // arguments and trailing optional arguments may be omitted. All required
+ // arguments must be supplied. Will fail if a required argument is missing or
+ // if an invalid type is passed. In this case, sets an exception on the
+ // context and returns false. Returns true otherwise.
+ bool GetArguments(int argc, JsArgument *argv);
// Get the type of an argument that was passed in.
JsParamType GetArgumentType(int i);
==== //depot/googleclient/gears/opensource/gears/blob/blob.cc#10 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/blob/blob.cc ====
# action=edit type=text
--- googleclient/gears/opensource/gears/blob/blob.cc 2008-09-04
12:40:23.000000000 +0100
+++ googleclient/gears/opensource/gears/blob/blob.cc 2008-09-03
16:30:48.000000000 +0100
@@ -108,14 +108,16 @@
{ JSPARAM_REQUIRED, JSPARAM_INT64, &offset },
{ JSPARAM_OPTIONAL, JSPARAM_INT64, &length },
};
- int argc = context->GetArguments(ARRAYSIZE(argv), argv);
- if (context->is_exception_set()) return;
+ if (!context->GetArguments(ARRAYSIZE(argv), argv)) {
+ assert(context->is_exception_set());
+ return;
+ }
if (offset < 0) {
context->SetException(STRING16(L"Offset must be a non-negative integer."));
return;
}
- if (argc != 2) {
+ if (!argv[1].was_specified) {
int64 blob_size = contents_->Length();
length = (blob_size > offset) ? blob_size - offset : 0;
}
==== //depot/googleclient/gears/opensource/gears/canvas/canvas.cc#16 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/canvas/canvas.cc ====
# action=edit type=text
--- googleclient/gears/opensource/gears/canvas/canvas.cc 2008-09-04
12:40:23.000000000 +0100
+++ googleclient/gears/opensource/gears/canvas/canvas.cc 2008-09-03
17:37:56.000000000 +0100
@@ -106,9 +106,10 @@
{ JSPARAM_OPTIONAL, JSPARAM_STRING16, &mime_type },
{ JSPARAM_OPTIONAL, JSPARAM_OBJECT, &attributes }
};
- int num_arguments = context->GetArguments(ARRAYSIZE(args), args);
- if (context->is_exception_set())
- return;
+ if (!context->GetArguments(ARRAYSIZE(args), args)) {
+ assert(context->is_exception_set());
+ return;
+ }
SkImageEncoder::Type type;
if (mime_type == STRING16(L"") ||
StringCompareIgnoreCase(mime_type.c_str(), STRING16(L"image/png")) == 0)
{
@@ -165,7 +166,8 @@
scoped_ptr<SkImageEncoder> encoder(SkImageEncoder::Create(type));
bool encode_succeeded;
double quality;
- if (num_arguments == 2 &&
+ if (args[0].was_specified && // Only use attributes if mime_types was also
+ args[1].was_specified && // specified.
attributes.GetPropertyAsDouble(STRING16(L"quality"), &quality)) {
if (quality < 0.0 || quality > 1.0) {
context->SetException(STRING16(L"quality must be between 0.0 and 1.0"));
====
//depot/googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.cc#13
-
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.cc
2008-09-04 12:40:23.000000000 +0100
+++ googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.cc
2008-09-03 16:55:31.000000000 +0100
@@ -412,9 +412,10 @@
{ JSPARAM_OPTIONAL, JSPARAM_INT, &dest_width },
{ JSPARAM_OPTIONAL, JSPARAM_INT, &dest_height }
};
- int num_arguments = context->GetArguments(ARRAYSIZE(args), args);
- if (context->is_exception_set())
- return;
+ if (!context->GetArguments(ARRAYSIZE(args), args)) {
+ assert(context->is_exception_set());
+ return;
+ }
assert(other_module);
if (GearsCanvas::kModuleName != other_module->get_module_name()) {
context->SetException(STRING16(L"Argument must be a Canvas."));
@@ -422,12 +423,23 @@
}
scoped_refptr<GearsCanvas> src = static_cast<GearsCanvas*>(other_module);
- if (num_arguments != 9) {
+ bool optional_source_pos_arguments_present = args[1].was_specified &&
+ args[2].was_specified;
+ bool optional_source_size_arguments_present = args[3].was_specified &&
+ args[4].was_specified;
+ bool optional_dest_arguments_present = args[5].was_specified &&
+ args[6].was_specified &&
+ args[7].was_specified &&
+ args[8].was_specified;
+ if (!(optional_source_pos_arguments_present &&
+ optional_source_size_arguments_present &&
+ optional_dest_arguments_present)) {
// Handle missing arguments.
- if (num_arguments == 5) {
+ if (optional_source_pos_arguments_present &&
+ optional_source_size_arguments_present) {
dest_width = source_width;
dest_height = source_height;
- } else if (num_arguments == 3) {
+ } else if (optional_source_pos_arguments_present) {
dest_width = src->width();
dest_height = src->height();
} else {
==== //depot/googleclient/gears/opensource/gears/cctests/test.cc#78 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/cctests/test.cc ====
# action=edit type=text
--- googleclient/gears/opensource/gears/cctests/test.cc 2008-09-04
12:40:24.000000000 +0100
+++ googleclient/gears/opensource/gears/cctests/test.cc 2008-09-04
11:49:20.000000000 +0100
@@ -444,7 +444,8 @@
}
void GearsTest::TestPassArgumentsOptional(JsCallContext *context) {
- int int_values[3] = {};
+ static const int max_num_arguments = 3;
+ int int_values[max_num_arguments] = {};
JsArgument argv[] = {
{JSPARAM_REQUIRED, JSPARAM_INT, &(int_values[0])},
@@ -452,16 +453,20 @@
{JSPARAM_OPTIONAL, JSPARAM_INT, &(int_values[2])}
};
- int argc = context->GetArguments(ARRAYSIZE(argv), argv);
- if (context->is_exception_set()) return;
-
- for (int i = 0; i < argc; ++i) {
- if (int_values[i] != 42) {
- std::string16 error(STRING16(L"Incorrect value for parameter "));
- error += IntegerToString16(i + 1);
- error += STRING16(L".");
- context->SetException(error);
- return;
+ if (!context->GetArguments(ARRAYSIZE(argv), argv)) {
+ assert(context->is_exception_set());
+ return;
+ }
+
+ for (int i = 0; i < max_num_arguments; ++i) {
+ if (argv[i].was_specified) {
+ if (int_values[i] != 42) {
+ std::string16 error(STRING16(L"Incorrect value for parameter "));
+ error += IntegerToString16(i + 1);
+ error += STRING16(L".");
+ context->SetException(error);
+ return;
+ }
}
}
}
@@ -704,9 +709,7 @@
void GearsTest::TestGetType(JsCallContext *context) {
// Don't really care about the actual value of the second parameter. We
// specify an argument of type JSPARAM_TOKEN because all types (other than
- // NULL and undefined) can be cast to this type by GetArguments. In these two
- // cases, GetArguments will not parse the argument (it is optional) and will
- // return 1, rather than 2.
+ // NULL and undefined) can be cast to this type by GetArguments.
std::string16 type;
JsToken value;
JsArgument argv[] = {
@@ -2044,7 +2047,7 @@
JsArgument argv[] = {
{ JSPARAM_REQUIRED, JSPARAM_ARRAY, &js_array }
};
- if (context->GetArguments(ARRAYSIZE(argv), argv) != ARRAYSIZE(argv)) {
+ if (!context->GetArguments(ARRAYSIZE(argv), argv)) {
assert(context->is_exception_set());
return;
}
==== //depot/googleclient/gears/opensource/gears/console/console.cc#3 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/console/console.cc ====
# action=edit type=xtext
--- googleclient/gears/opensource/gears/console/console.cc 2008-09-04
12:40:24.000000000 +0100
+++ googleclient/gears/opensource/gears/console/console.cc 2008-09-03
17:02:19.000000000 +0100
@@ -51,10 +51,10 @@
{ JSPARAM_REQUIRED, JSPARAM_STRING16, &message },
{ JSPARAM_OPTIONAL, JSPARAM_ARRAY, &args_array},
};
- int argc = context->GetArguments(ARRAYSIZE(argv), argv);
-
- if (context->is_exception_set())
+ if (!context->GetArguments(ARRAYSIZE(argv), argv)) {
+ assert(context->is_exception_set());
return;
+ }
// Check input validity.
if (type_str.length() == 0) {
@@ -68,7 +68,7 @@
}
std::string16 msg = message;
- if (argc == 3) {
+ if (argv[2].was_specified) {
InterpolateArgs(&message, &args_array);
}
LogEvent *log_event = new LogEvent(message, type_str, EnvPageLocationUrl());
==== //depot/googleclient/gears/opensource/gears/database/database.cc#5 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/database/database.cc ====
# action=edit type=text
--- googleclient/gears/opensource/gears/database/database.cc 2008-09-04
12:40:24.000000000 +0100
+++ googleclient/gears/opensource/gears/database/database.cc 2008-09-03
17:13:24.000000000 +0100
@@ -138,9 +138,10 @@
{ JSPARAM_REQUIRED, JSPARAM_STRING16, &expr },
{ JSPARAM_OPTIONAL, JSPARAM_ARRAY, &arg_array },
};
- int argc = context->GetArguments(ARRAYSIZE(argv), argv);
- if (context->is_exception_set())
- return;
+ if (!context->GetArguments(ARRAYSIZE(argv), argv)) {
+ assert(context->is_exception_set());
+ return;
+ }
#ifdef BROWSER_IE
LOG16((L"DB Execute: %s\n", expr));
@@ -169,7 +170,7 @@
}
// Bind parameters
- if (!BindArgsToStatement(context, (argc >= 2) ? &arg_array : NULL,
+ if (!BindArgsToStatement(context, argv[1].was_specified ? &arg_array : NULL,
stmt.get())) {
// BindArgsToStatement already set an exception
return;
==== //depot/googleclient/gears/opensource/gears/database2/transaction.cc#6 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/database2/transaction.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/database2/transaction.cc
2008-09-04 12:40:24.000000000 +0100
+++ googleclient/gears/opensource/gears/database2/transaction.cc
2008-09-03 17:52:54.000000000 +0100
@@ -99,7 +99,7 @@
{ JSPARAM_OPTIONAL, JSPARAM_FUNCTION, &temp_error_callback }
};
- int argc = context->GetArguments(ARRAYSIZE(argv), argv);
+ context->GetArguments(ARRAYSIZE(argv), argv);
JsArray *sql_arguments = &temp_sql_arguments;
scoped_ptr<JsRootedCallback> callback(temp_callback);
scoped_ptr<JsRootedCallback> error_callback(temp_error_callback);
@@ -112,13 +112,19 @@
// if any of the arguments are not supplied or null, send them to statement
// factory as NULL
- if (argc < 2 || JsTokenIsNullOrUndefined(sql_arguments->token())) {
+ if (!argv[1].was_specified ||
+ JsTokenIsNullOrUndefined(sql_arguments->token())) {
sql_arguments = NULL;
}
- if (argc < 3 || JsTokenIsNullOrUndefined(callback->token())) {
+ if (!argv[1].was_specified || // Don't use temp_callback unless
+ !argv[2].was_specified || // temp_sql_arguments was also specified.
+ JsTokenIsNullOrUndefined(callback->token())) {
callback.reset(NULL);
}
- if (argc < 4 || JsTokenIsNullOrUndefined(error_callback->token())) {
+ if (!argv[1].was_specified || // Don't use temp_error_callback unless
+ !argv[2].was_specified || // temp_callback and temp_sql_arguments were
+ !argv[3].was_specified || // also specified.
+ JsTokenIsNullOrUndefined(error_callback->token())) {
error_callback.reset(NULL);
}
==== //depot/googleclient/gears/opensource/gears/desktop/desktop.cc#47 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/desktop/desktop.cc ====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/desktop.cc 2008-09-04
12:40:24.000000000 +0100
+++ googleclient/gears/opensource/gears/desktop/desktop.cc 2008-09-04
11:46:41.000000000 +0100
@@ -422,13 +422,15 @@
{ JSPARAM_REQUIRED, JSPARAM_FUNCTION, as_out_parameter(callback) },
{ JSPARAM_OPTIONAL, JSPARAM_OBJECT, &options_map },
};
- int argc = context->GetArguments(ARRAYSIZE(argv), argv);
- if (context->is_exception_set()) return;
+ if (!context->GetArguments(ARRAYSIZE(argv), argv)) {
+ assert(context->is_exception_set());
+ return;
+ }
// TODO(cdevries): set focus to tab where this function was called
FileDialog::Options options;
- if (argc > 1) {
+ if (argv[1].was_specified) {
if (!FileDialog::ParseOptions(context, options_map, &options)) {
assert(context->is_exception_set());
return;
==== //depot/googleclient/gears/opensource/gears/dummy/dummy_module.cc#2 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/dummy/dummy_module.cc ====
# action=edit type=text
--- googleclient/gears/opensource/gears/dummy/dummy_module.cc 2008-09-04
12:40:24.000000000 +0100
+++ googleclient/gears/opensource/gears/dummy/dummy_module.cc 2008-09-03
17:26:26.000000000 +0100
@@ -62,16 +62,16 @@
};
// Read arguments.
- int argument_count = context->GetArguments(ARRAYSIZE(argv), argv);
+ context->GetArguments(ARRAYSIZE(argv), argv);
// Context will throw an exception if required arguments are missing or
// argument types are mismatched. Check for this and return if exception has
// been thrown.
if (context->is_exception_set()) return;
- // You can check for number of arguments with which the method was called and
- // adjust your logic accordingly.
- if (argument_count < 2) {
+ // You can check which optional arguments were supplied and adjust your logic
+ // accordingly.
+ if (!argv[1].was_specified) {
second_argument = -1;
}
==== //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#46
-
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/geolocation/geolocation.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/geolocation.cc
2008-09-02 19:02:14.000000000 +0100
+++ googleclient/gears/opensource/gears/geolocation/geolocation.cc
2008-09-04 12:21:50.000000000 +0100
@@ -752,7 +752,7 @@
// Arguments are: function successCallback, optional function errorCallback,
// optional object options. errorCallback can be null.
//
- // Note that GetArgumentsForGeolocation allocates a new JsRootedCallback.
+ // Note that GetArguments allocates a new JsRootedCallback.
//
JsRootedCallback *success_callback = NULL;
JsRootedCallback *error_callback = NULL;
@@ -763,7 +763,7 @@
{ JSPARAM_OPTIONAL, JSPARAM_OBJECT, &options, false },
};
- bool success = context->GetArguments2(ARRAYSIZE(argv), argv);
+ bool success = context->GetArguments(ARRAYSIZE(argv), argv);
if (!success) {
delete success_callback;
delete error_callback;
====
//depot/googleclient/gears/opensource/gears/test/testcases/database2_tests.js#4
-
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/test/testcases/database2_tests.js
====
# action=edit type=text
--- googleclient/gears/opensource/gears/test/testcases/database2_tests.js
2008-09-04 12:40:24.000000000 +0100
+++ googleclient/gears/opensource/gears/test/testcases/database2_tests.js
2008-09-04 12:35:34.000000000 +0100
@@ -102,7 +102,7 @@
function testDatabaseVersionNull() {
assertError(function() {
var db = db_manager.openDatabase('unit_test_db', null);
- }, "Required argument 2 is missing.");
+ }, "Null or undefined passed for required argument 2.");
}
function testDatabaseApiSig() {
====
//depot/googleclient/gears/opensource/gears/test/testcases/internal_tests.js#33
-
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/test/testcases/internal_tests.js
====
# action=edit type=text
--- googleclient/gears/opensource/gears/test/testcases/internal_tests.js
2008-09-04 00:11:20.000000000 +0100
+++ googleclient/gears/opensource/gears/test/testcases/internal_tests.js
2008-09-04 12:27:45.000000000 +0100
@@ -68,8 +68,9 @@
"Incorrect value for parameter " + (i + 1));
}
- // Test not passing required args
- var required_argument_error = "Required argument 5 is missing.";
+ // Test passing null and undefined for required args
+ var required_argument_error =
+ "Null or undefined passed for required argument 5.";
assertError(function() {
internalTests.testPassArguments(good_bool, good_int, good_int64,
good_double, null); },
@@ -78,10 +79,12 @@
internalTests.testPassArguments(good_bool, good_int, good_int64,
good_double, undefined); },
required_argument_error);
+
+ // Test not passing required args
assertError(function() {
internalTests.testPassArguments(good_bool, good_int, good_int64,
good_double); },
- required_argument_error);
+ "Required argument 5 is missing");
// Test passing wrong type
for (var i = 0; i < good_vals.length; i++) {
@@ -135,6 +138,9 @@
internalTests.testPassArgumentsOptional(42, 42, 42);
internalTests.testPassArgumentsOptional(42, 42);
internalTests.testPassArgumentsOptional(42);
+ internalTests.testPassArgumentsOptional(42, null, null);
+ internalTests.testPassArgumentsOptional(42, null);
+ internalTests.testPassArgumentsOptional(42, null, 42);
assertError(
function() { internalTests.testPassArgumentsOptional(42, "hello"); },
====
//depot/googleclient/gears/opensource/gears/ui/common/permissions_dialog.cc#6 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/ui/common/permissions_dialog.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/ui/common/permissions_dialog.cc
2008-09-04 12:40:24.000000000 +0100
+++ googleclient/gears/opensource/gears/ui/common/permissions_dialog.cc
2008-09-03 18:12:53.000000000 +0100
@@ -153,6 +153,16 @@
if (context->is_exception_set())
return NULL;
+ // Ignore image_url if site_name was not provided.
+ if (!argv[0].was_specified) {
+ image_url = STRING16(L"");
+ }
+
+ // Ignore extra_message if image_url or site_name was not provided.
+ if (!argv[0].was_specified || !argv[1].was_specified) {
+ extra_message = STRING16(L"");
+ }
+
return new CustomContent(image_url.c_str(),
site_name.c_str(),
extra_message.c_str());
