Hello cprince,
I'd like you to do a code review. Please execute
g4 diff -c 8175690
or point your web browser to
http://mondrian/8175690
to review the following code:
Change 8175690 by [EMAIL PROTECTED] on 2008/09/04 16:14:27 *pending*
Enable the leak-counting GUI on IE, and also plug an IE6 memory leak
in the unit test code. This brings us to no known leaks on the unit
test at tester/gui.html, for IE6/WinXP.
R=cprince
[EMAIL PROTECTED]
DELTA=23 (22 added, 0 deleted, 1 changed)
OCL=8175690
Affected files ...
... //depot/googleclient/gears/opensource/gears/base/common/leak_counter.h#7
edit
... //depot/googleclient/gears/opensource/gears/base/ie/module.cc#3 edit
... //depot/googleclient/gears/opensource/gears/test/tester/harness.js#7 edit
23 delta lines: 22 added, 0 deleted, 1 changed
Also consider running:
g4 lint -c 8175690
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 8175690 by [EMAIL PROTECTED] on 2008/09/04 16:14:27 *pending*
Enable the leak-counting GUI on IE, and also plug an IE6 memory leak
in the unit test code. This brings us to no known leaks on the unit
test at tester/gui.html, for IE6/WinXP.
Affected files ...
... //depot/googleclient/gears/opensource/gears/base/common/leak_counter.h#7
edit
... //depot/googleclient/gears/opensource/gears/base/ie/module.cc#3 edit
... //depot/googleclient/gears/opensource/gears/test/tester/harness.js#7 edit
==== //depot/googleclient/gears/opensource/gears/base/common/leak_counter.h#7 -
c:\devel\srcwingears2/googleclient/gears/opensource/gears/base/common/leak_counter.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/leak_counter.h
2008-08-18 17:33:00.000000000 +1000
+++ googleclient/gears/opensource/gears/base/common/leak_counter.h
2008-09-01 11:58:04.000000000 +1000
@@ -27,10 +27,15 @@
#define GEARS_BASE_COMMON_LEAK_COUNTER_H__
#ifdef DEBUG
-#if BROWSER_FF
+#if BROWSER_FF || BROWSER_IE
+#ifdef WINCE
+// TODO(nigeltao): figure out some sort of UI for showing leaks on WinCE.
+#else
#define ENABLE_LEAK_COUNTING 1
#endif
#endif
+#endif
+
#if ENABLE_LEAK_COUNTING
==== //depot/googleclient/gears/opensource/gears/base/ie/module.cc#3 -
c:\devel\srcwingears2/googleclient/gears/opensource/gears/base/ie/module.cc ====
# action=edit type=xtext
--- googleclient/gears/opensource/gears/base/ie/module.cc 2008-09-01
12:18:45.000000000 +1000
+++ googleclient/gears/opensource/gears/base/ie/module.cc 2008-09-01
12:15:15.000000000 +1000
@@ -23,6 +23,7 @@
// OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
// ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#include "gears/base/common/leak_counter.h"
#include "gears/base/common/process_utils_win32.h"
#include "gears/base/common/thread_locals.h"
#include "gears/base/ie/resource.h" // for .rgs resource ids (IDR_*)
@@ -39,6 +40,14 @@
DECLARE_LIBID(LIBID_GearsTypelib)
DECLARE_REGISTRY_APPID_RESOURCEID(IDR_GEARSIE, \
"{B56936F7-0433-4E0B-921B-D095E7142B6D}")
+
+ DllModule() {
+ LEAK_COUNTER_INITIALIZE();
+ }
+
+ ~DllModule() {
+ LEAK_COUNTER_DUMP_COUNTS();
+ }
};
DllModule atl_module;
==== //depot/googleclient/gears/opensource/gears/test/tester/harness.js#7 -
c:\devel\srcwingears2/googleclient/gears/opensource/gears/test/tester/harness.js
====
# action=edit type=text
--- googleclient/gears/opensource/gears/test/tester/harness.js 2008-09-04
16:10:53.000000000 +1000
+++ googleclient/gears/opensource/gears/test/tester/harness.js 2008-09-04
16:15:20.000000000 +1000
@@ -191,6 +191,14 @@
google.gears.workerPool.onerror = this.workerHandleGlobalError_;
} else {
window.onerror = this.handleGlobalError_;
+ // On IE6, at least, assigning window.onerror leads to a circular COM
+ // reference, and hence a memory leak. Presumably the circular reference
+ // is between between this (an instance of Harness) and the global object
+ // called window. To break this circular reference, we reset window.onerror
+ // on page unload.
+ window.onunload = function() {
+ window.onerror = null;
+ }
}
// Run the first test directly; no need to schedule.
