Hello bpm,
I'd like you to do a code review. Please execute
g4 diff -c 8214903
or point your web browser to
http://mondrian/8214903
to review the following code:
Change 8214903 by [EMAIL PROTECTED] on 2008/09/09 21:42:22 *pending*
Fix crasher when the GearsDesktop JavaScript object gets
garbage collected before the file dialog closes.
PRESUBMIT=passed
R=bpm
[EMAIL PROTECTED]
DELTA=7 (1 added, 0 deleted, 6 changed)
OCL=8214903
Affected files ...
... //depot/googleclient/gears/opensource/gears/desktop/file_dialog.cc#9 edit
... //depot/googleclient/gears/opensource/gears/desktop/file_dialog.h#6 edit
7 delta lines: 1 added, 0 deleted, 6 changed
Also consider running:
g4 lint -c 8214903
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 8214903 by [EMAIL PROTECTED] on 2008/09/09 21:42:22 *pending*
Fix crasher when the GearsDesktop JavaScript object gets
garbage collected before the file dialog closes.
Affected files ...
... //depot/googleclient/gears/opensource/gears/desktop/file_dialog.cc#9 edit
... //depot/googleclient/gears/opensource/gears/desktop/file_dialog.h#6 edit
==== //depot/googleclient/gears/opensource/gears/desktop/file_dialog.cc#9 -
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/desktop/file_dialog.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/file_dialog.cc 2008-09-05
09:32:55.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/file_dialog.cc 2008-09-09
21:39:38.000000000 +1000
@@ -89,7 +89,7 @@
Mutex FileDialog::active_mutex_;
FileDialog::ActiveMap FileDialog::active_;
-FileDialog* FileDialog::Create(const ModuleImplBaseClass* module) {
+FileDialog* FileDialog::Create(ModuleImplBaseClass* module) {
NativeWindowPtr parent = NULL;
GetBrowserWindow(module, &parent);
#if defined(WIN32)
@@ -109,9 +109,9 @@
FileDialog::FileDialog() {
}
-void FileDialog::Init(const ModuleImplBaseClass* module,
+void FileDialog::Init(ModuleImplBaseClass* module,
NativeWindowPtr parent) {
- module_ = module;
+ module_.reset(module);
parent_ = parent;
}
==== //depot/googleclient/gears/opensource/gears/desktop/file_dialog.h#6 -
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/desktop/file_dialog.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/file_dialog.h 2008-09-09
21:36:34.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/file_dialog.h 2008-09-09
21:39:39.000000000 +1000
@@ -32,6 +32,7 @@
#include "gears/base/common/common.h"
#include "gears/base/common/js_types.h"
#include "gears/base/common/mutex.h"
+#include "gears/base/common/scoped_refptr.h"
#include "gears/ui/common/window_utils.h"
#include "third_party/scoped_ptr/scoped_ptr.h"
@@ -59,7 +60,7 @@
StringList filter;
};
- static FileDialog* Create(const ModuleImplBaseClass* module);
+ static FileDialog* Create(ModuleImplBaseClass* module);
virtual ~FileDialog();
@@ -97,7 +98,7 @@
FileDialog();
- void Init(const ModuleImplBaseClass* module, NativeWindowPtr parent);
+ void Init(ModuleImplBaseClass* module, NativeWindowPtr parent);
// Implemented per platform to create and display a dialog with the provided
// options, or returns false and sets error. The dialog displays
@@ -125,7 +126,7 @@
private:
typedef std::map<ModuleEnvironment*, FileDialog*> ActiveMap;
- const ModuleImplBaseClass* module_;
+ scoped_refptr<ModuleImplBaseClass> module_;
NativeWindowPtr parent_;
scoped_ptr<JsRootedCallback> callback_;
