---------- Forwarded message ---------- From: Noel Gordon <[email protected]> Date: 2009/5/17 Subject: Re: a medium-size code review (11167428) Read files only once per window entry, rath To: Nigel Tao <[email protected]> Cc: [email protected]
Agree it's getting a little less than pretty, let's start with a few things I noticed ... ======================================================================== http://mondrian.corp.google.com/file/11167428///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc?a=1 File //depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc (snapshot 1) ------------------------------------ Line 334: // TODO(nigeltao): Should we early exit for DRAG_AND_DROP_EVENT_DRAGLEAVE This comment can maybe go? Unless we still have concerns about FF on OSX? aka GetDragAndDropEventType() does all the hard work we need now, right? ------------------------------------ Line 353: // TODO(nigeltao): cache the filenames (and their MIME types, extensions, This comment needs some love, speed < one minute on it. ------------------------------------ Line 447: if (NS_FAILED(nr)) { success = false; break; } Just noticed, no check for a directory here, no link following, etc ... ------------------------------------ Line 548: if (type == DRAG_AND_DROP_EVENT_DRAGLEAVE) { We can remove this DRAG_AND_DROP_EVENT_DRAGLEAVE check now, and hence be consistent with the other drivers? ------------------------------------ Line 603: continue; early out here, "return false;" ? ------------------------------------ Line 630: module_environment->drop_target_interceptor_->SetCachedMetaDataIsValid(true); The GetFileDragAndDropMetaData() call @628 also sets the meta data state valid state internally. SetCachedMetaDataIsValid() subsequently forces the meta data state valid to true. Why? ======================================================================== -- To respond, reply to this email or visit http://mondrian.corp.google.com/11167428
