========================================================================
http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/base/safari/browser_load_hook.mm?a=1
File 
//depot/googleclient/gears/opensource/gears/base/safari/browser_load_hook.mm 
(snapshot 1)
------------------------------------
Line 113: SwizzleWebViewMethods();
On Mon Nov 10 09:28:46 2008 PST, playmobil wrote:
> We often get support emails from people who have problems with Gears in Safari
> due around the initialization process.  This function has paranoid logging for
> this reason, could you add LOG() statements before and after this function and
> everywhere else that might seem important.

Done.
========================================================================
http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.h?a=1
File 
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.h 
(snapshot 1)
------------------------------------
Line 30: #include "gears/base/common/string16.h"
On Mon Nov 10 09:30:39 2008 PST, playmobil wrote:
> Could you add some comments here on why we need this and where exactly each of
> these methods is Swizzled into the WebView hierarchy.

I've added this comment:
// Gears Drag and Drop needs to get access to more than what regular JavaScript
// driven drag and drop can see (and hence, what JavaScript via the NPAPI
// interface can see). In particular, for file drag and drop, this includes
// things like the list of filenames during drag enter/over events (not just
// on drop events) and also whether or not we are in a genuine (i.e. OS-level)
// drag and drop event, and not a programatically generated, synthetic event.
// In order to get access, we intercept certain methods on the WebView class.
// We can do this (via a technique called "Method Swizzling") since a
// WebView is an Objective-C class, not a C++ class, and we can re-wire what
// code gets run in response to drag-related messages (i.e. selectors, in
// Objective-C parlance).
// This SwizzleWebViewMethods function sets up that interception, and the
// other functions in this file are more or less getters for what we learn
// when those intercepts are triggered.
========================================================================
http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm?a=2
File 
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm 
(snapshot 2)
------------------------------------
Line 57: void MethodSwizzle(Class klass, SEL old_selector, SEL new_selector) {
On Mon Nov 10 09:35:20 2008 PST, playmobil wrote:
> Please add paranoid logging here.
> 
> Any errors should use NSLog() so we can see the output in release versions.
> 
> This function should return a bool with success/failure.

Done.
------------------------------------
Line 86: for (int i = 0; i < numberOfFiles; ++i) {
On Mon Nov 10 09:38:21 2008 PST, playmobil wrote:
> Use an enumerator here:
>
http://developer.apple.com/documentation/Cocoa/Reference/Foundation/Classes/NSArray_Class/Reference/Reference.html#//apple_ref/occ/instm/NSArray/objectEnumerator

Done.
------------------------------------
Line 94: return [self swizzledDraggingEntered:draggingInfo];
On Mon Nov 10 09:39:59 2008 PST, playmobil wrote:
> Are you sure it's ok that both you and the WebView are handling these events?

I am not a Cocoa expert, but I believe that it's OK. All I'm doing here is
calling some getters - I'm not modifying the Pasteboard, or the return value of
the default implementation.
------------------------------------
Line 117: void SwizzleWebViewMethods() {
On Mon Nov 10 09:42:27 2008 PST, playmobil wrote:
> This should return a bool success/failure.

Done.
========================================================================
http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/desktop/drop_target_sf.h?a=1
File //depot/googleclient/gears/opensource/gears/desktop/drop_target_sf.h 
(snapshot 1)
------------------------------------
Line 92:
On Mon Nov 10 09:43:47 2008 PST, playmobil wrote:
> extra whitespace.

Done.
========================================================================

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

Reply via email to