Hello mpcomplete,

I'd like you to do a code review.  Please execute
        g4 diff -c 9468733

or point your web browser to
        http://mondrian/9468733

to review the following code:

Change 9468733 by z...@zkuznia-corp3 on 2008/12/18 11:31:50 *pending*

        Fixing File Selection dialog to work in the Chrome renderer process
        
        R=mpcomplete
        [email protected]
        DELTA=132  (129 added, 0 deleted, 3 changed)
        OCL=9468733

Affected files ...

... //depot/googleclient/gears/opensource/gears/base/chrome/module_cr.cc#3 edit
... //depot/googleclient/gears/opensource/gears/base/chrome/module_cr.h#2 edit
... //depot/googleclient/gears/opensource/gears/desktop/file_dialog.cc#17 edit
... //depot/googleclient/gears/opensource/gears/desktop/file_dialog_win32.cc#11 
edit
... //depot/googleclient/gears/opensource/gears/desktop/file_dialog_win32.h#8 
edit
... 
//depot/googleclient/gears/opensource/third_party/chrome/chrome_plugin_api.h#3 
edit

132 delta lines: 129 added, 0 deleted, 3 changed

Also consider running:
        g4 lint -c 9468733

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 9468733 by z...@zkuznia-corp3 on 2008/12/18 11:31:50 *pending*

        Fixing File Selection dialog to work in the Chrome renderer process

Affected files ...

... //depot/googleclient/gears/opensource/gears/base/chrome/module_cr.cc#3 edit
... //depot/googleclient/gears/opensource/gears/base/chrome/module_cr.h#2 edit
... //depot/googleclient/gears/opensource/gears/desktop/file_dialog.cc#17 edit
... //depot/googleclient/gears/opensource/gears/desktop/file_dialog_win32.cc#11 
edit
... //depot/googleclient/gears/opensource/gears/desktop/file_dialog_win32.h#8 
edit
... 
//depot/googleclient/gears/opensource/gears/desktop/shortcut_utils_win32.cc#12 
edit
... 
//depot/googleclient/gears/opensource/third_party/chrome/chrome_plugin_api.h#3 
edit

==== //depot/googleclient/gears/opensource/gears/base/chrome/module_cr.cc#3 - 
c:\src-gears3/googleclient/gears/opensource/gears/base/chrome/module_cr.cc ====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/chrome/module_cr.cc        
2008-12-18 11:28:03.000000000 -0800
+++ googleclient/gears/opensource/gears/base/chrome/module_cr.cc        
2008-12-18 11:03:25.000000000 -0800
@@ -230,6 +230,24 @@
   }
 }
 
+void STDCALL CPP_OnFileDialogResult(void *data, uint32 result,
+                                    const char **files, uint32 files_len) {
+  CPFileDialog *dialog_info = static_cast<CPFileDialog *>(data);
+
+  if (result) {
+    std::vector<std::string16> filenames;
+    std::string16 filename;
+    for (uint32 c = 0; c < files_len; c++) {
+      UTF8ToString16(files[c], &filename);
+      filenames.push_back(filename);
+    }
+
+    dialog_info->OnSelect(filenames);
+  } else {
+    dialog_info->OnCancel();
+  }
+}
+
 void STDCALL CPP_HtmlDialogClosed(void *plugin_context,
                                   const char *json_retval) {
   assert(plugin_context);
@@ -384,6 +402,7 @@
   plugin_funcs.should_intercept_request = CPP_ShouldInterceptRequest;
   plugin_funcs.on_message = CPP_OnMessage;
   plugin_funcs.on_sync_message = CPP_OnSyncMessage;
+  plugin_funcs.on_file_dialog_result = CPP_OnFileDialogResult;
   plugin_funcs.html_dialog_closed = CPP_HtmlDialogClosed;
   plugin_funcs.handle_command = CPP_HandleCommand;
   size_t pfuncs_size = pfuncs->size;
@@ -562,6 +581,20 @@
   }
 }
 
+bool CPFileDialog::OpenDialog(uint32 type, const char16 *filter) {
+  std::string filter_utf8;
+  if (!String16ToUTF8(filter, wcslen(filter), &filter_utf8)) {
+    return false;
+  }
+
+
+  return (CPERR_SUCCESS ==
+          CP::browser_funcs().open_file_dialog(g_cpid,
+                                               type,
+                                               filter_utf8.c_str(),
+                                               this));
+}
+
 CPBrowsingContext CP::GetBrowsingContext(JsContextPtr context) {
   // TODO(mpcomplete): This only works for the plugin process.  We need to
   // support something like this in the browser process as well.
==== //depot/googleclient/gears/opensource/gears/base/chrome/module_cr.h#2 - 
c:\src-gears3/googleclient/gears/opensource/gears/base/chrome/module_cr.h ====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/chrome/module_cr.h 2008-12-18 
11:28:03.000000000 -0800
+++ googleclient/gears/opensource/gears/base/chrome/module_cr.h 2008-12-18 
11:02:59.000000000 -0800
@@ -195,4 +195,13 @@
  protected:
   std::vector<uint8> retval_;
 };
+
+// This class can only be used on the plugin thread
+class CPFileDialog {
+ public:
+  bool OpenDialog(uint32 type,
+                  const char16 *filter);
+  virtual void OnSelect(const std::vector<std::string16> &files) = 0;
+  virtual void OnCancel() {}
+};
 #endif  // GEARS_BASE_CHROME_MODULE_CR_H__
==== //depot/googleclient/gears/opensource/gears/desktop/file_dialog.cc#17 - 
c:\src-gears3/googleclient/gears/opensource/gears/desktop/file_dialog.cc ====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/file_dialog.cc  2008-12-18 
11:28:03.000000000 -0800
+++ googleclient/gears/opensource/gears/desktop/file_dialog.cc  2008-12-17 
10:43:21.000000000 -0800
@@ -25,6 +25,9 @@
 
 #include "gears/desktop/file_dialog.h"
 
+#if BROWSER_CHROME
+#include "gears/base/chrome/module_cr.h"
+#endif
 #include "gears/base/common/base_class.h"
 #include "gears/base/common/js_types.h"
 #include "gears/base/common/scoped_refptr.h"
@@ -95,7 +98,16 @@
   NativeWindowPtr parent = NULL;
   GetBrowserWindow(module_environment, &parent);
 #if defined(WIN32)
+#if BROWSER_CHROME
+  FileDialog* dialog = NULL;
+  if (CP::gears_in_renderer()) {
+    dialog = new FileDialogChrome;
+  } else {
+    dialog = new FileDialogWin32;
+  }
+#else
   FileDialog* dialog = new FileDialogWin32;
+#endif
 #elif defined(OS_MACOSX)
   FileDialog* dialog = new FileDialogCarbon;
 #elif defined(LINUX)
==== 
//depot/googleclient/gears/opensource/gears/desktop/file_dialog_win32.cc#11 - 
c:\src-gears3/googleclient/gears/opensource/gears/desktop/file_dialog_win32.cc 
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/file_dialog_win32.cc    
2008-12-18 11:28:03.000000000 -0800
+++ googleclient/gears/opensource/gears/desktop/file_dialog_win32.cc    
2008-12-18 11:29:38.000000000 -0800
@@ -30,6 +30,33 @@
 #include "gears/base/common/string_utils.h"
 #include "gears/ui/common/i18n_strings.h"
 
+#if BROWSER_CHROME
+
+bool FileDialogChrome::BeginSelection(NativeWindowPtr parent,
+                                      const FileDialog::Options& options,
+                                      std::string16* error) {
+  std::string16 default_filter;
+  for (StringList::const_iterator it = options.filter.begin();
+       it != options.filter.end();
+       ++it) {
+    // Handle extensions of the form ".foo".
+    if (L'.' == (*it)[0]) {
+      default_filter.append(L";*");
+      default_filter.append(*it);
+      continue;
+    }
+  }
+  return OpenDialog(2, default_filter.c_str());
+}
+
+void FileDialogChrome::CancelSelection() {
+}
+
+void FileDialogChrome::DoUIAction(UIAction action) {
+}
+
+#endif  // BROWSER_CHROME
+
 namespace {
 
 const char16 kSelectionCompleteTopic[] = L"selection complete";
@@ -214,6 +241,8 @@
       LONG result = RegEnumKeyEx(HKEY_CLASSES_ROOT, index, name, &len, NULL,
                                  NULL, NULL, NULL);
       if (ERROR_NO_MORE_ITEMS == result)
+        break;
+      if (ERROR_INVALID_HANDLE == result)
         break;
       if (ERROR_SUCCESS != result)
         continue;
==== //depot/googleclient/gears/opensource/gears/desktop/file_dialog_win32.h#8 
- c:\src-gears3/googleclient/gears/opensource/gears/desktop/file_dialog_win32.h 
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/file_dialog_win32.h     
2008-12-18 11:28:03.000000000 -0800
+++ googleclient/gears/opensource/gears/desktop/file_dialog_win32.h     
2008-12-17 10:45:41.000000000 -0800
@@ -32,6 +32,27 @@
 #include "gears/base/common/message_service.h"
 #include "gears/base/common/thread.h"
 #include "gears/desktop/file_dialog.h"
+
+#if BROWSER_CHROME
+#include "gears/base/chrome/module_cr.h"
+
+class FileDialogChrome : public CPFileDialog, public FileDialog {
+ public:
+  void OnSelect(const std::vector<std::string16> &files) {
+    CompleteSelection(files);
+  }
+  void OnCancel() {
+  }
+
+  bool BeginSelection(NativeWindowPtr parent,
+                      const FileDialog::Options& options,
+                      std::string16* error);
+  void CancelSelection();
+  void DoUIAction(UIAction action);
+ private:
+};
+
+#endif  // BROWSER_CHROME
 
 class JsRunnerInterface;
 
==== 
//depot/googleclient/gears/opensource/gears/desktop/shortcut_utils_win32.cc#12 
- 
c:\src-gears3/googleclient/gears/opensource/gears/desktop/shortcut_utils_win32.cc
 ====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/shortcut_utils_win32.cc 
2008-12-03 10:27:17.000000000 -0800
+++ googleclient/gears/opensource/gears/desktop/shortcut_utils_win32.cc 
2008-12-03 11:18:49.000000000 -0800
@@ -268,8 +268,13 @@
 // given URL.
 #if BROWSER_CHROME
 static std::string16 GetArgumentsString(const std::string16 &url)  {
+  std::string16 new_url = url;
+  ReplaceAll(new_url, std::string16(STRING16(L"%")),
+             std::string16(STRING16(L"\\%")));
+
   std::string url_utf8;
-  String16ToUTF8(url.c_str(), &url_utf8);
+  String16ToUTF8(new_url.c_str(), &url_utf8);
+  //url_utf8 = EscapeUrl(url_utf8, 0);
   char* arguments_utf8 = NULL;
   bool rv = g_cpbrowser_funcs.get_command_line_arguments(
       g_cpid, NULL, url_utf8.c_str(), &arguments_utf8) == CPERR_SUCCESS;
@@ -284,7 +289,16 @@
 }
 #else
 static inline std::string16 GetArgumentsString(const std::string16 &url)  {
-  return url;
+  /*std::string url_utf8;
+  String16ToUTF8(url.c_str(), &url_utf8);
+  url_utf8 = EscapeUrl(url_utf8, 0);*/
+
+  std::string16 new_url = url;
+  ReplaceAll(new_url, std::string16(STRING16(L"%")),
+             std::string16(STRING16(L"\\%")));
+//  UTF8ToString16(url_utf8, &new_url);
+
+  return new_url;
 }
 #endif
 
==== 
//depot/googleclient/gears/opensource/third_party/chrome/chrome_plugin_api.h#3 
- 
c:\src-gears3/googleclient/gears/opensource/third_party/chrome/chrome_plugin_api.h
 ====
# action=edit type=text
--- googleclient/gears/opensource/third_party/chrome/chrome_plugin_api.h        
2008-12-18 11:28:03.000000000 -0800
+++ googleclient/gears/opensource/third_party/chrome/chrome_plugin_api.h        
2008-12-18 11:03:39.000000000 -0800
@@ -59,7 +59,7 @@
 // The current version of the API, used by the 'version' field of CPPluginFuncs
 // and CPBrowserFuncs.
 #define CP_MAJOR_VERSION 0
-#define CP_MINOR_VERSION 8
+#define CP_MINOR_VERSION 9
 #define CP_VERSION       ((CP_MAJOR_VERSION << 8) | (CP_MINOR_VERSION))
 
 #define CP_GET_MAJOR_VERSION(version) ((version & 0xff00) >> 8)
@@ -419,12 +419,21 @@
                                                          void (*func)(void *),
                                                          void *user_data);
 
+typedef CPError (STDCALL *CPB_OpenFileDialogFunc)(CPID id,
+                                                  uint32 type,
+                                                  const char *filter,
+                                                  void *user_data);
+
 // Informs the plugin of raw data having been sent from another process.
 typedef void (STDCALL *CPP_OnMessageFunc)(void *data, uint32 data_len);
 
 // Informs the plugin of raw data having been sent from another process.
 typedef void (STDCALL *CPP_OnSyncMessageFunc)(void *data, uint32 data_len,
                                               void **retval, uint32 
*retval_len);
+
+typedef void (STDCALL *CPP_OnFileDialogResultFunc)(void *data, uint32 result,
+                                                   const char **files,
+                                                   uint32 files_len);
 
 // Function table for issuing requests using via the other side's network 
stack.
 // For the plugin, this functions deal with issuing requests through the
@@ -469,6 +478,7 @@
   CPP_HtmlDialogClosedFunc html_dialog_closed;
   CPP_HandleCommandFunc handle_command;
   CPP_OnSyncMessageFunc on_sync_message;
+  CPP_OnFileDialogResultFunc on_file_dialog_result;
 } CPPluginFuncs;
 
 // Function table CPB functions (functions provided by host to plugin).
@@ -498,6 +508,7 @@
   CPB_HandleCommandFunc handle_command;
   CPB_SendSyncMessageFunc send_sync_message;
   CPB_PluginThreadAsyncCallFunc plugin_thread_async_call;
+  CPB_OpenFileDialogFunc open_file_dialog;
 } CPBrowserFuncs;
 
 

Reply via email to