A few minor comments. ======================================================================== http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/base/safari/browser_load_hook.mm?a=2 File //depot/googleclient/gears/opensource/gears/base/safari/browser_load_hook.mm (snapshot 2) ------------------------------------ Line 115: LOG(("Gears: Swizzling WebView methods failed")); This should be a NSLog(@"") so we can see the output in release builds if swizzling fails. ======================================================================== http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.h?a=2 File //depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.h (snapshot 2) ------------------------------------ Line 44: // when those intercepts are triggered. That's great, another thing you might mention is which functions are swizzled, that we only track state, always call the original function and don't modify the return values. ======================================================================== http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm?a=3 File //depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm (snapshot 3) ------------------------------------ Line 74: result ? @"succeeded" : @"failed"); This should be a LOG(), you can NSLog on failure, but it's fine by me if you just NSLog() in the calling method and LOG() here. ------------------------------------ Line 124: return nit: may be a little cleaner to stick these in a vector and iterate it. ======================================================================== http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/desktop/drop_target_base.h?a=1 File //depot/googleclient/gears/opensource/gears/desktop/drop_target_base.h (snapshot 1) ------------------------------------ Line 33: (BROWSER_WEBKIT && defined(OS_MACOSX)) BROWSER_WEBKIT is enough here, you don't need the &&. ======================================================================== http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/desktop/drop_target_sf.cc?a=1 File //depot/googleclient/gears/opensource/gears/desktop/drop_target_sf.cc (snapshot 1) ------------------------------------ Line 111: // indirect workaround. You may want to file a WebKit bug if it's in their code, would be nice to know the limitations of their NPAPI implementation... ------------------------------------ Line 116: if (NPN_GetValue(npp, NPNVWindowNPObject, &global) != NPERR_NO_ERROR) { You can use js_runner->GetGlobalObject() to get access to the window object. ------------------------------------ Line 287: drop_target->Ref(); // Balanced by an Unref() call during UnregisterSelf. I know it's Google style to minimize NL's, but a newline after this line would help make it stand out... ========================================================================
-- To respond, reply to this email or visit http://mondrian.corp.google.com/8931781
