========================================================================
http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/base/safari/browser_load_hook.mm?a=3
File 
//depot/googleclient/gears/opensource/gears/base/safari/browser_load_hook.mm 
(snapshot 3)
------------------------------------
Line 115: LOG(("Gears: Swizzling WebView methods failed"));
On Tue Nov 11 08:59:03 2008 PST, playmobil wrote:
> This should be a NSLog(@"") so we can see the output in release builds if
> swizzling fails.

Really? The rest of the function uses LOG instead of NSLog. But, digging into
base/common/common_osx.mm, LOG is a Gears macro that just ends up calling NSLog.
========================================================================
http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.h?a=3
File 
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.h 
(snapshot 3)
------------------------------------
Line 44: // when those intercepts are triggered.
On Tue Nov 11 09:00:51 2008 PST, playmobil wrote:
> 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.

Done.
========================================================================
http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm?a=5
File 
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm 
(snapshot 5)
------------------------------------
Line 74: result ? @"succeeded" : @"failed");
On Tue Nov 11 09:03:29 2008 PST, playmobil wrote:
> 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.

As previously mentioned, LOG eventually calls NSLog, so unless I'm confused,
there's no practical difference to what you're suggesting.
------------------------------------
Line 124: return
On Tue Nov 11 09:12:56 2008 PST, playmobil wrote:
> nit: may be a little cleaner to stick these in a vector and iterate it.

Eh, maybe, but its marginal.
========================================================================
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))
On Tue Nov 11 09:14:29 2008 PST, playmobil wrote:
> BROWSER_WEBKIT is enough here, you don't need the &&.

Does Chromium not define BROWSER_WEBKIT?
========================================================================
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.
On Tue Nov 11 09:24:10 2008 PST, playmobil wrote:
> 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...

I'm not sure if it's a bug in WebKit, in the Gears / NPAPI bridge code, or just
in the way I was using the latter. Whatever the cause, I'm sure this is fixable,
it's just that it would involve some more tinkering, and I would rather just
check in (and hence can work on in parallel more easily, in multiple clients)
something working and iterate.
------------------------------------
Line 116: if (NPN_GetValue(npp, NPNVWindowNPObject, &global) != NPERR_NO_ERROR) 
{
On Tue Nov 11 09:19:35 2008 PST, playmobil wrote:
> You can use js_runner->GetGlobalObject() to get access to the window object.

Done, which did require adding its declaration to base/common/js_runner.h.
------------------------------------
Line 287: drop_target->Ref();  // Balanced by an Unref() call during 
UnregisterSelf.
On Tue Nov 11 09:23:02 2008 PST, playmobil wrote:
> I know it's Google style to minimize NL's, but a newline after this line would
> help make it stand out...

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

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

Reply via email to