========================================================================
http://mondrian.corp.google.com/file/10892071///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_cr.cc?a=1
File
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_cr.cc
(snapshot 1)
------------------------------------
Line 43: #define ELEMENT_EVENT_DRAGENTER 1
Can you point me to what parts of the chromium code correspond to these magic
#defines?
------------------------------------
Line 64: event_ = NPVARIANT_TO_OBJECT(event->token());
Do you need to AddRef event_ here (and Release, or whatever the NPAPI equivalent
is, in the DragSession constructor)?
------------------------------------
Line 138: class FileAttributes {
Can you add a TODO to refactor this and the similar code in
drag_and_drop_utils_ie.cc? The result should probably live in
drag_and_drop_utils_win32.cc.
------------------------------------
Line 176: #if defined(DEBUG)
I would just put the #if around the call to DisplayAttributes on line 191,
rather than making a separate #define DebugAttributes.
------------------------------------
Line 272: if (std::string(drag->type()) != "Files")
Can "Files" be "application/x-gears-files"? What parts of the chromium codebase
do I have to look at to understand the context for this changelist?
------------------------------------
Line 292: for (size_t i = 0; i < g_drag_files; ++i) {
In order to be consistent with the other browsers (and to avoid duplicate code),
please just call the FileDragAndDropMetaData::SetFilenames method, from
drag_and_drop_utils_common (and hence replace the g_mime_set, g_type_set,
etcetera by a FileDragAndDropMetaData instance, as the _ff, _ie and _sf ports
do). Similarly, your AddFileMetaData and FileDialog::FilesToJsObjectArray calls
below more or less become a method call to FileDragAndDropMetaData::ToJsObject.
If there are bugs or cross-platform inconsistencies with
FileDragAndDropMetaData, such as with how it handles directories, or whether an
empty set of files should be represented by undefined or by a 0-length array,
then either patch drag_and_drop_utils_common.cc or add TODOs.
------------------------------------
Line 382: if (!g_identity || g_identity < drag.identity()) {
Is identity strictly increasing? If so, would sequence_number be a better name?
Or should the "<" instead be a "!="?
Again, what parts of the chromium codebase do I have to look at to understand
the context for this changelist?
========================================================================
--
To respond, reply to this email or visit
http://mondrian.corp.google.com/10892071
