Hello noel,
I'd like you to do a code review. Please execute
g4 diff -c 10876520
or point your web browser to
http://mondrian/10876520
to review the following code:
Change 10876520 by nigel...@nigeltao-srcgears5 on 2009/04/21 17:44:47 *pending*
Remove desktop.acceptDrag, since it is deprecated in favor of
desktop.setDragCursor. Also, introduce desktop.setDropEffect as
an alias for desktop.setDragCursor.
PRESUBMIT=passed
R=noel
[email protected]
DELTA=125 (2 added, 122 deleted, 1 changed)
OCL=10876520
Affected files ...
... //depot/googleclient/gears/opensource/gears/desktop/desktop.cc#85 edit
... //depot/googleclient/gears/opensource/gears/desktop/desktop.h#31 edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc#15
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.h#8
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.cc#13
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.h#8
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.h#8
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.mm#11
edit
125 delta lines: 2 added, 122 deleted, 1 changed
Also consider running:
g4 lint -c 10876520
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 10876520 by nigel...@nigeltao-srcgears5 on 2009/04/21 17:44:47 *pending*
Remove desktop.acceptDrag, since it is deprecated in favor of
desktop.setDragCursor. Also, introduce desktop.setDropEffect as
an alias for desktop.setDragCursor.
Affected files ...
... //depot/googleclient/gears/opensource/gears/desktop/desktop.cc#85 edit
... //depot/googleclient/gears/opensource/gears/desktop/desktop.h#31 edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc#15
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.h#8
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.cc#13
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.h#8
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.h#8
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.mm#11
edit
==== //depot/googleclient/gears/opensource/gears/desktop/desktop.cc#85 -
/home/nigeltao/srcgears5/googleclient/gears/opensource/gears/desktop/desktop.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/desktop.cc 2009-04-21
17:44:54.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/desktop.cc 2009-04-21
17:38:47.000000000 +1000
@@ -76,11 +76,11 @@
// The Drag-and-Drop API has not been finalized for official builds.
#else
#if GEARS_DRAG_AND_DROP_API_IS_SUPPORTED_FOR_THIS_PLATFORM
- // TODO(nigeltao): should acceptDrag be renamed finishDrag??
- RegisterMethod("acceptDrag", &GearsDesktop::AcceptDrag);
RegisterMethod("extractMetaData", &GearsDesktop::ExtractMetaData);
RegisterMethod("getDragData", &GearsDesktop::GetDragData);
+ // TODO(nigeltao): Decide on exactly one of setDragCursor and SetDropEffect.
RegisterMethod("setDragCursor", &GearsDesktop::SetDragCursor);
+ RegisterMethod("setDropEffect", &GearsDesktop::SetDragCursor);
#endif
#endif // OFFICIAL_BUILD
}
@@ -875,40 +875,6 @@
// The Drag-and-Drop API has not been finalized for official builds.
#else
#if GEARS_DRAG_AND_DROP_API_IS_SUPPORTED_FOR_THIS_PLATFORM
-void GearsDesktop::AcceptDrag(JsCallContext *context) {
- if (EnvIsWorker()) {
- context->SetException(
- STRING16(L"acceptDrag is not supported in workers."));
- return;
- }
-
- scoped_ptr<JsObject> event_as_js_object;
- bool acceptance;
- JsArgument argv[] = {
- { JSPARAM_REQUIRED, JSPARAM_OBJECT, &event_as_js_object },
- { JSPARAM_REQUIRED, JSPARAM_BOOL, &acceptance },
- };
- context->GetArguments(ARRAYSIZE(argv), argv);
- if (context->is_exception_set()) return;
-
- std::string16 error;
-#if BROWSER_FF || BROWSER_IE || BROWSER_WEBKIT
- ::AcceptDrag(module_environment_.get(),
- event_as_js_object.get(),
- acceptance,
- &error);
-#else
- // TODO(nigeltao): implement on Chromium.
- error = STRING16(L"acceptDrag is not supported for this platform.");
-#endif
-
- if (!error.empty()) {
- context->SetException(error);
- return;
- }
-}
-
-
void GearsDesktop::ExtractMetaData(JsCallContext *context) {
ModuleImplBaseClass *other_module = NULL;
==== //depot/googleclient/gears/opensource/gears/desktop/desktop.h#31 -
/home/nigeltao/srcgears5/googleclient/gears/opensource/gears/desktop/desktop.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/desktop.h 2009-04-21
17:44:54.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/desktop.h 2009-04-21
17:37:10.000000000 +1000
@@ -173,17 +173,6 @@
#if defined(OFFICIAL_BUILD) || defined(OS_ANDROID)
// The Drag-and-Drop API has not been finalized for official builds.
#else
- // TODO(nigeltao): decide which of AcceptDrag and SetDragCursor to
- // keep and which to cull, since they essentially do the same thing (except
- // that AcceptDrag takes responsibility for calling event.cancelBubble(true),
- // event.returnValue=false, event.dataTransfer.dropEffect="copy", whereas
- // the SetDragCursor model only does the latter of those three, and puts
- // the onus for the first two on the web-app (rather than Gears)).
-
- // IN: Event event, bool acceptance
- // OUT: -
- void AcceptDrag(JsCallContext *context);
-
// IN: GearsBlob blob
// OUT: object metadata
void ExtractMetaData(JsCallContext *context);
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc#15
-
/home/nigeltao/srcgears5/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc
2009-04-21 17:44:54.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc
2009-04-21 17:41:11.000000000 +1000
@@ -264,26 +264,6 @@
}
-void AcceptDrag(ModuleEnvironment *module_environment,
- JsObject *event_as_js_object,
- bool acceptance,
- std::string16 *error_out) {
- DragAndDropCursorType cursor_type = acceptance
- ? DRAG_AND_DROP_CURSOR_COPY
- : DRAG_AND_DROP_CURSOR_NONE;
- SetDragCursor(module_environment, event_as_js_object, cursor_type,
error_out);
- if (!error_out->empty()) {
- return;
- }
- nsCOMPtr<nsIDOMEvent> dom_event;
- DragAndDropEventType type = GetDragAndDropEventType(
- module_environment, event_as_js_object, &dom_event);
- if (type == DRAG_AND_DROP_EVENT_DROP) {
- dom_event->StopPropagation();
- }
-}
-
-
void SetDragCursor(ModuleEnvironment *module_environment,
JsObject *event_as_js_object,
DragAndDropCursorType cursor_type,
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.h#8
-
/home/nigeltao/srcgears5/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.h
2009-04-21 17:44:54.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.h
2009-04-21 17:41:14.000000000 +1000
@@ -40,11 +40,6 @@
JsObject *data_out,
std::string16 *error_out);
-void AcceptDrag(ModuleEnvironment *module_environment,
- JsObject *event,
- bool acceptance,
- std::string16 *error_out);
-
bool GetDragData(ModuleEnvironment *module_environment,
JsObject *event,
JsObject *data_out,
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.cc#13
-
/home/nigeltao/srcgears5/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.cc
2009-04-21 17:44:55.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.cc
2009-04-21 17:42:11.000000000 +1000
@@ -177,33 +177,6 @@
}
-void AcceptDrag(ModuleEnvironment *module_environment,
- JsObject *event,
- bool acceptance,
- std::string16 *error_out) {
- DragAndDropCursorType cursor_type = acceptance
- ? DRAG_AND_DROP_CURSOR_COPY
- : DRAG_AND_DROP_CURSOR_NONE;
- SetDragCursor(module_environment, event, cursor_type, error_out);
- if (!error_out->empty()) {
- return;
- }
- CComPtr<IHTMLEventObj> window_event;
- DragAndDropEventType type = GetWindowEvent(module_environment, window_event);
- if (type != DRAG_AND_DROP_EVENT_DROP) {
- CComQIPtr<IHTMLEventObj2> window_event2(window_event);
- CComPtr<IHTMLDataTransfer> data_transfer;
- if (!window_event2 ||
- FAILED(window_event->put_returnValue(CComVariant(false))) ||
- FAILED(window_event->put_cancelBubble(VARIANT_TRUE)) ||
- FAILED(window_event2->get_dataTransfer(&data_transfer)) ||
- FAILED(data_transfer->put_dropEffect(acceptance ? L"copy" : L"none")))
{
- *error_out = GET_INTERNAL_ERROR_MESSAGE();
- return;
- }
- }
-}
-
void SetDragCursor(ModuleEnvironment *module_environment,
JsObject *event,
DragAndDropCursorType cursor_type,
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.h#8
-
/home/nigeltao/srcgears5/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.h
2009-04-21 17:44:55.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.h
2009-04-21 17:41:49.000000000 +1000
@@ -38,11 +38,6 @@
bool AddFileDragAndDropData(ModuleEnvironment *module_environment,
FileDragAndDropMetaData *meta_data_out);
-void AcceptDrag(ModuleEnvironment *module_environment,
- JsObject *event,
- bool acceptance,
- std::string16 *error_out);
-
bool GetDragData(ModuleEnvironment *module_environment,
JsObject *event,
JsObject *data_out,
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.h#8
-
/home/nigeltao/srcgears5/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.h
2009-04-21 17:44:55.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.h
2009-04-21 17:42:37.000000000 +1000
@@ -59,11 +59,6 @@
bool IsInADragOperation();
bool IsInADropOperation();
-void AcceptDrag(ModuleEnvironment *module_environment,
- JsObject *event,
- bool acceptance,
- std::string16 *error_out);
-
bool GetDragData(ModuleEnvironment *module_environment,
JsObject *event,
JsObject *data_out,
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.mm#11
-
/home/nigeltao/srcgears5/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.mm
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.mm
2009-04-21 17:44:55.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.mm
2009-04-21 17:43:24.000000000 +1000
@@ -39,7 +39,7 @@
static bool g_is_in_a_drag_operation = false;
static bool g_is_in_a_drop_operation = false;
// The next two variables are for when the page's JavaScript explicitly
-// accepts or rejects the drag, via desktop.acceptDrag(..., bool).
+// sets the drop effect (i.e. cursor), via desktop.setDropEffect(..., bool).
// In either case, g_drag_operation_has_been_set becomes true, and
// g_drag_operation takes NSDragOperationCopy or NSDragOperationNone.
static bool g_drag_operation_has_been_set = false;
@@ -228,19 +228,6 @@
module_environment, IsInADropOperation(), data_out, error_out);
};
-void AcceptDrag(ModuleEnvironment *module_environment,
- JsObject *event,
- bool acceptance,
- std::string16 *error_out) {
- DragAndDropCursorType cursor_type = acceptance
- ? DRAG_AND_DROP_CURSOR_COPY
- : DRAG_AND_DROP_CURSOR_NONE;
- SetDragCursor(module_environment, event, cursor_type, error_out);
- if (error_out->empty()) {
- event->SetPropertyBool(STRING16(L"returnValue"), false);
- }
-}
-
bool GetDragData(ModuleEnvironment *module_environment,
JsObject *event_as_js_object,
JsObject *data_out,
