========================================================================
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
I get the same questions from the chromium side; someone has to go first, so can
we?  The chromes like using 0 for none or invalid values, and in other places
they like -1.  In gears, the invalid drag event invalid value is at the tail of
the enum list.

I'd like to use the defines from desktop/drag_and_drop_utils_common.h - if we
can start that gears-defined event enum from 1, then I could remove the defines
here and that'd be ok in chrome and for the other browsers we support in gears I
think.
------------------------------------
Line 64: event_ = NPVARIANT_TO_OBJECT(event->token());
No not needed.
------------------------------------
Line 138: class FileAttributes {
Yeap, TODO added.
------------------------------------
Line 176: #if defined(DEBUG)
probably not worth it, I plan to ditch this code.
------------------------------------
Line 272: if (std::string(drag->type()) != "Files")
We discussed this before, the "Files" is internal between chrome and gears (it's
never exposed to user code) and I looked for some short name that makes sense,
like "Files".
------------------------------------
Line 292: for (size_t i = 0; i < g_drag_files; ++i) {
If you compared the AddFileMetaData here to the one in V1, note that I rewrote
it here closer to FileDragAndDropMetaData and added the error checking.  So I'm
heading in the direction of FileDragAndDropMetaData, but the inconsistencies in
that code you mentioned means it doesn't quite work for me.  I'm chasing a
crasher in chrome, for example, and the directory handling is helping me debug
that issue, so I'd rather not remove it.  We have TODOs for the
FileDragAndDropMetaData cross-platform inconsistencies on our site page.
------------------------------------
Line 382: if (!g_identity || g_identity < drag.identity()) {
On 4:54 pm, 23 Apr, nigeltao wrote:
> Is identity strictly increasing? If so, would sequence_number be a better
name?

Tony was ok with identity, so maybe that'll do.  He wanted it initialized to -1,
and turned into an int (that's what they do with ids).  I wanted an uint32
because yes it's strictly increasing from the gears point of view.  But our
coding standard says we can't use uint32 for that, so we compromised on an
strictly increasing int32!  Maybe not such a great compromise on my part, I now
have to remember to cast it uint32 in chrome when returning it to gears on the
CPAPI to cover it up.

> Or should the "<" instead be a "!="?

We need the "<", they want int32, gears wants uint32, oh well, someone has to go
first,
http://codereview.chromium.org/28108.  I'd like to cement my CPAPI changes into
chrome, but just yesterday we were reconsidering drop effects on that API.  And
I think I 'll need to go rejig 
http://codereview.chromium.org/67297 as a result.
========================================================================

-- 
To respond, reply to this email or visit 
http://mondrian.corp.google.com/10892071

Reply via email to