Hello noel,
I'd like you to do a code review. Please execute
g4 diff -c 9147404
or point your web browser to
http://mondrian/9147404
to review the following code:
Change 9147404 by [EMAIL PROTECTED] on 2008/11/25 15:40:24 *pending*
Rename s/osx/sf/ in drag_and_drop_utils_osx.{cc,h}. The code inside
is more Safari-specific than OS X specific.
PRESUBMIT=passed
R=noel
[EMAIL PROTECTED]
DELTA=473 (235 added, 235 deleted, 3 changed)
OCL=9147404
Affected files ...
... //depot/googleclient/gears/opensource/gears/Makefile#214 edit
...
//depot/googleclient/gears/opensource/gears/base/safari/browser_load_hook.mm#14
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.h#2
delete
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm#2
delete
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.h#1
add
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.mm#1
add
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_sf.cc#2 edit
473 delta lines: 235 added, 235 deleted, 3 changed
Also consider running:
g4 lint -c 9147404
which verifies that the changelist doesn't introduce new style violations.
If you can't do the review, please let me know as soon as possible. During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately. Visit
http://www/eng/code_review.html for more information.
This is a semiautomated message from "g4 mail". Complaints or suggestions?
Mail [EMAIL PROTECTED]
Change 9147404 by [EMAIL PROTECTED] on 2008/11/25 15:40:24 *pending*
Rename s/osx/sf/ in drag_and_drop_utils_osx.{cc,h}. The code inside
is more Safari-specific than OS X specific.
Affected files ...
... //depot/googleclient/gears/opensource/gears/Makefile#214 edit
...
//depot/googleclient/gears/opensource/gears/base/safari/browser_load_hook.mm#14
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.h#2
delete
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm#2
delete
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.h#1
add
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.mm#1
add
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_sf.cc#2 edit
==== //depot/googleclient/gears/opensource/gears/Makefile#214 -
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/Makefile ====
# action=edit type=text
--- googleclient/gears/opensource/gears/Makefile 2008-11-25
15:39:26.000000000 +1100
+++ googleclient/gears/opensource/gears/Makefile 2008-11-25
15:38:10.000000000 +1100
@@ -1498,7 +1498,7 @@
# The Drag-and-Drop API has not been finalized for official builds.
else
SF_CPPSRCS += \
- drag_and_drop_utils_osx.mm \
+ drag_and_drop_utils_sf.mm \
drop_target_sf.cc \
$(NULL)
endif # not OFFICIAL_BUILD
====
//depot/googleclient/gears/opensource/gears/base/safari/browser_load_hook.mm#14
-
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/base/safari/browser_load_hook.mm
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/safari/browser_load_hook.mm
2008-11-25 15:39:26.000000000 +1100
+++ googleclient/gears/opensource/gears/base/safari/browser_load_hook.mm
2008-11-25 15:38:00.000000000 +1100
@@ -30,7 +30,7 @@
#import "gears/base/common/exception_handler.h"
#import "gears/base/common/message_queue.h"
#import "gears/base/safari/safari_workarounds.h"
-#import "gears/desktop/drag_and_drop_utils_osx.h"
+#import "gears/desktop/drag_and_drop_utils_sf.h"
#import "gears/localserver/safari/http_handler.h"
#import "gears/localserver/safari/ui_thread.h"
#import "gears/ui/safari/settings_menu.h"
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.h#2
- None ====
# action=delete type=text
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm#2
- None ====
# action=delete type=text
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.h#1
-
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.h
====
# action=add type=text
--- /dev/null 1970-01-01 10:00:00.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.h
2008-11-25 15:39:04.000000000 +1100
@@ -0,0 +1,61 @@
+// Copyright 2008, Google Inc.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+// 1. Redistributions of source code must retain the above copyright notice,
+// this list of conditions and the following disclaimer.
+// 2. Redistributions in binary form must reproduce the above copyright
notice,
+// this list of conditions and the following disclaimer in the
documentation
+// and/or other materials provided with the distribution.
+// 3. Neither the name of Google Inc. nor the names of its contributors may be
+// used to endorse or promote products derived from this software without
+// specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED
+// WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
+// EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
+// OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+// OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+// ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#ifndef GEARS_DESKTOP_DRAG_AND_DROP_UTILS_SF_H__
+#define GEARS_DESKTOP_DRAG_AND_DROP_UTILS_SF_H__
+
+#include "gears/base/common/base_class.h"
+
+// 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.
+// Generallly speaking, method swizzling is a bit of a hack that could
+// destabilize the application, but our swizzled methods only track state,
+// they always call the original function, and the don't modify the return
+// values. See the .mm implementation file for a list of what methods we
+// actually swizzle.
+bool SwizzleWebViewMethods();
+
+bool GetDroppedFiles(ModuleEnvironment *module_environment,
+ JsArray *files_out,
+ bool reset);
+
+// These two are mutually exclusive (although they may be both false) -
+// "in a drag" means dragenter, dragover or dragleave, but not drop.
+bool IsInADragOperation();
+bool IsInADropOperation();
+
+#endif // GEARS_DESKTOP_DRAG_AND_DROP_UTILS_SF_H__
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.mm#1
-
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.mm
====
# action=add type=text
--- /dev/null 1970-01-01 10:00:00.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_sf.mm
2008-11-25 15:39:14.000000000 +1100
@@ -0,0 +1,174 @@
+// Copyright 2008, Google Inc.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+// 1. Redistributions of source code must retain the above copyright notice,
+// this list of conditions and the following disclaimer.
+// 2. Redistributions in binary form must reproduce the above copyright
notice,
+// this list of conditions and the following disclaimer in the
documentation
+// and/or other materials provided with the distribution.
+// 3. Neither the name of Google Inc. nor the names of its contributors may be
+// used to endorse or promote products derived from this software without
+// specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED
+// WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
+// EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
+// OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+// OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+// ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#import <Cocoa/Cocoa.h>
+#import <WebKit/DOM.h>
+#import <WebKit/WebView.h>
+#import <objc/objc-class.h>
+#import <vector>
+
+#import "gears/desktop/drag_and_drop_utils_sf.h"
+
+#import "gears/base/common/common.h"
+#import "gears/base/safari/nsstring_utils.h"
+#import "gears/desktop/file_dialog.h"
+
+static std::vector<std::string16> g_dragging_pasteboard_filenames_;
+static bool g_is_in_a_drag_operation_ = false;
+static bool g_is_in_a_drop_operation_ = false;
+
+bool IsInADragOperation() {
+ return g_is_in_a_drag_operation_;
+}
+
+bool IsInADropOperation() {
+ return g_is_in_a_drop_operation_;
+}
+
+// We "swizzle" some Cocoa method implementations to insert a little code
+// before and after certain events get processed. For more, see
+// http://www.cocoadev.com/index.pl?MethodSwizzling
+bool MethodSwizzle(Class klass, SEL old_selector, SEL new_selector) {
+ Method old_method = class_getInstanceMethod(klass, old_selector);
+ Method new_method = class_getInstanceMethod(klass, new_selector);
+ bool result = false;
+ if (old_method && new_method) {
+ char *temp1 = old_method->method_types;
+ old_method->method_types = new_method->method_types;
+ new_method->method_types = temp1;
+
+ IMP temp2 = old_method->method_imp;
+ old_method->method_imp = new_method->method_imp;
+ new_method->method_imp = temp2;
+ result = true;
+ }
+#ifdef DEBUG
+ NSLog(@"Gears: Swizzling \"[EMAIL PROTECTED]" for class %@ %@",
+ NSStringFromSelector(old_selector),
+ NSStringFromClass(klass),
+ result ? @"succeeded" : @"failed");
+#endif
+ return result;
+}
+
[EMAIL PROTECTED] WebView (GearsSwizzledMethods)
+- (NSDragOperation)swizzledDraggingEntered:(id <NSDraggingInfo>)draggingInfo;
+- (NSDragOperation)swizzledDraggingUpdated:(id <NSDraggingInfo>)draggingInfo;
+- (NSDragOperation)swizzledDraggingExited:(id <NSDraggingInfo>)draggingInfo;
+- (BOOL)swizzledPerformDragOperation:(id <NSDraggingInfo>)draggingInfo;
[EMAIL PROTECTED]
+
[EMAIL PROTECTED] WebView (GearsSwizzledMethods)
+
+- (NSDragOperation)swizzledDraggingEntered:(id <NSDraggingInfo>)draggingInfo {
+ g_dragging_pasteboard_filenames_.clear();
+
+ // In Safari, arbitrary web pages can put on the pasteboard during an ondrag
+ // event, simply by calling window.event.dataTransfer.setData('URL',
+ // 'file:///some/file'). If we did not distinguish such drag sources, then
+ // a malicious web page could access the local file system, with the help of
+ // Gears drag and drop, by tricking a user to drag one image on the page and
+ // dropping it on another image on the same page.
+ // To prevent this, we require that the source of the drag be from a
+ // separate application (i.e. it is in a separate process' address
+ // space), and hence [draggingInfo draggingSource] will return nil rather
+ // than a pointer to something in the address space of this Safari process
+ // (or more specifically, a pointer to something caused by a web page viewed
+ // in Safari). If Safari ever went to a multi-process architecture, then we
+ // would have to revisit this guard.
+ BOOL is_drag_from_another_application =
+ ([draggingInfo draggingSource] == nil);
+
+ if (is_drag_from_another_application) {
+ NSPasteboard *pboard = [draggingInfo draggingPasteboard];
+ if ([[pboard types] containsObject:NSFilenamesPboardType]) {
+ NSArray *files = [pboard propertyListForType:NSFilenamesPboardType];
+ NSEnumerator *enumerator = [files objectEnumerator];
+ NSString *ns_string;
+ while ((ns_string = [enumerator nextObject])) {
+ std::string16 std_string;
+ [ns_string string16:&std_string];
+ g_dragging_pasteboard_filenames_.push_back(std_string);
+ }
+ }
+ }
+
+ return [self swizzledDraggingEntered:draggingInfo];
+}
+
+- (NSDragOperation)swizzledDraggingUpdated:(id <NSDraggingInfo>)draggingInfo {
+ g_is_in_a_drag_operation_ = true;
+ NSDragOperation result = [self swizzledDraggingUpdated:draggingInfo];
+ g_is_in_a_drag_operation_ = false;
+ return result;
+}
+
+- (NSDragOperation)swizzledDraggingExited:(id <NSDraggingInfo>)draggingInfo {
+ g_dragging_pasteboard_filenames_.clear();
+ return [self swizzledDraggingExited:draggingInfo];
+}
+
+- (BOOL)swizzledPerformDragOperation:(id <NSDraggingInfo>)draggingInfo {
+ g_is_in_a_drop_operation_ = true;
+ BOOL result = [self swizzledPerformDragOperation:draggingInfo];
+ g_is_in_a_drop_operation_ = false;
+ return result;
+}
+
[EMAIL PROTECTED]
+
+bool SwizzleWebViewMethods() {
+ SEL selectors[] = {
+ @selector(draggingEntered:),
+ @selector(swizzledDraggingEntered:),
+ @selector(draggingUpdated:),
+ @selector(swizzledDraggingUpdated:),
+ @selector(draggingExited:),
+ @selector(swizzledDraggingExited:),
+ @selector(performDragOperation:),
+ @selector(swizzledPerformDragOperation:),
+ };
+ for (unsigned int i = 0; i < ARRAYSIZE(selectors); i += 2) {
+ if (!MethodSwizzle([WebView class], selectors[i], selectors[i + 1])) {
+ return false;
+ }
+ }
+ return true;
+}
+
+bool GetDroppedFiles(ModuleEnvironment *module_environment,
+ JsArray *files_out,
+ bool reset) {
+ std::string16 ignored;
+ bool result = FileDialog::FilesToJsObjectArray(
+ g_dragging_pasteboard_filenames_,
+ module_environment,
+ files_out,
+ &ignored);
+ if (reset) {
+ g_dragging_pasteboard_filenames_.clear();
+ }
+ return result;
+};
==== //depot/googleclient/gears/opensource/gears/desktop/drop_target_sf.cc#2 -
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/desktop/drop_target_sf.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drop_target_sf.cc
2008-11-25 15:39:27.000000000 +1100
+++ googleclient/gears/opensource/gears/desktop/drop_target_sf.cc
2008-11-25 15:37:46.000000000 +1100
@@ -30,7 +30,7 @@
#include "gears/base/common/leak_counter.h"
#include "gears/base/npapi/scoped_npapi_handles.h"
-#include "gears/desktop/drag_and_drop_utils_osx.h"
+#include "gears/desktop/drag_and_drop_utils_sf.h"
#include "gears/desktop/file_dialog.h"
