========================================================================
http://mondrian.corp.google.com/file/10892071///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_common.h?a=1
File 
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_common.h
 (snapshot 1)
------------------------------------
Line 52: DRAG_AND_DROP_EVENT_DROP,
Strip the trailing comma.
========================================================================
http://mondrian.corp.google.com/file/10892071///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_cr.cc?a=5
File 
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_cr.cc 
(snapshot 5)
------------------------------------
Line 241: // "Say" there's 1 drag file until we know more; extract the files.
Do we really want 1 here, or is 0 more appropriate, in case we early exit (e.g.
if drag->GetDragData below fails)?
------------------------------------
Line 331: if (drag.BrowserSupportsDragDrop())
I feel better, with nested if's, when they all have {} curly brackets, even if
they are one-liners. But also, should the drag.BrowserSupportsDragDrop check
happen before you call drag.SetDropEffect? Is what you really want something
like:
if (!drag.BSDD || !d.SDE) {
  *error = ...
}
------------------------------------
Line 342: if (drag.BrowserSupportsDragDrop())
Same as @331.
========================================================================

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

Reply via email to