Hello steveblock,
I'd like you to do a code review. Please execute
g4 diff -c 8450696
or point your web browser to
http://mondrian/8450696
to review the following code:
Change 8450696 by [EMAIL PROTECTED] on 2008/10/01 18:54:33 *pending*
Add unload monitoring to WinCE
R=steveblock
[EMAIL PROTECTED]
DELTA=194 (176 added, 18 deleted, 0 changed)
OCL=8450696
Affected files ...
... //depot/googleclient/gears/opensource/gears/Makefile#185 edit
...
//depot/googleclient/gears/opensource/gears/base/common/html_event_monitor.cc#1
edit
...
//depot/googleclient/gears/opensource/gears/base/common/html_event_monitor.h#2
edit
...
//depot/googleclient/gears/opensource/gears/base/common/html_event_monitor_iemobile.cc#1
add
... //depot/googleclient/gears/opensource/gears/base/common/js_runner_ie.cc#19
edit
...
//depot/googleclient/gears/opensource/gears/base/common/wince_compatibility.cc#4
edit
...
//depot/googleclient/gears/opensource/gears/base/common/wince_compatibility.h#1
edit
... //depot/googleclient/gears/opensource/gears/factory/factory_impl.cc#17 edit
... //depot/googleclient/gears/opensource/gears/factory/factory_impl.h#4 edit
194 delta lines: 176 added, 18 deleted, 0 changed
Also consider running:
g4 lint -c 8450696
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 8450696 by [EMAIL PROTECTED] on 2008/10/01 18:54:33 *pending*
Add unload monitoring to WinCE
R=steveblock
[EMAIL PROTECTED]
DELTA=193 (175 added, 18 deleted, 0 changed)
OCL=8450696
Affected files ...
... //depot/googleclient/gears/opensource/gears/Makefile#185 edit
...
//depot/googleclient/gears/opensource/gears/base/common/html_event_monitor.cc#1
edit
...
//depot/googleclient/gears/opensource/gears/base/common/html_event_monitor.h#2
edit
...
//depot/googleclient/gears/opensource/gears/base/common/html_event_monitor_iemobile.cc#1
add
... //depot/googleclient/gears/opensource/gears/base/common/js_runner_ie.cc#19
edit
...
//depot/googleclient/gears/opensource/gears/base/common/wince_compatibility.cc#4
edit
...
//depot/googleclient/gears/opensource/gears/base/common/wince_compatibility.h#1
edit
... //depot/googleclient/gears/opensource/gears/factory/factory_impl.cc#17 edit
... //depot/googleclient/gears/opensource/gears/factory/factory_impl.h#4 edit
==== //depot/googleclient/gears/opensource/gears/Makefile#185 -
c:\GearsWinMo/googleclient/gears/opensource/gears/Makefile ====
# action=edit type=text
--- googleclient/gears/opensource/gears/Makefile 2008-10-01
19:07:30.000000000 +0100
+++ googleclient/gears/opensource/gears/Makefile 2008-10-01
14:06:30.000000000 +0100
@@ -1051,6 +1051,7 @@
IE_CPPSRCS += \
common_ie.cc \
file_wince.cc \
+ html_event_monitor_iemobile.cc \
wince_compatibility.cc \
$(NULL)
endif
====
//depot/googleclient/gears/opensource/gears/base/common/html_event_monitor.cc#1
-
c:\GearsWinMo/googleclient/gears/opensource/gears/base/common/html_event_monitor.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/html_event_monitor.cc
2008-10-01 19:07:30.000000000 +0100
+++ googleclient/gears/opensource/gears/base/common/html_event_monitor.cc
2008-10-01 18:36:03.000000000 +0100
@@ -23,15 +23,13 @@
// OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
// ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-#ifdef WINCE
-// This class does not work in WinCE, and is not referenced.
-#else
-
#include "gears/base/common/html_event_monitor.h"
+#ifdef WINCE
+#include "gears/base/common/wince_compatibility.h"
+#endif
const char16 *kEventUnload = STRING16(L"onunload");
-
HtmlEventMonitor::HtmlEventMonitor(const char16 *event_name,
HtmlEventCallback function,
@@ -48,4 +46,3 @@
}
}
-#endif // WINCE .. else
====
//depot/googleclient/gears/opensource/gears/base/common/html_event_monitor.h#2
-
c:\GearsWinMo/googleclient/gears/opensource/gears/base/common/html_event_monitor.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/html_event_monitor.h
2008-10-01 19:07:30.000000000 +0100
+++ googleclient/gears/opensource/gears/base/common/html_event_monitor.h
2008-10-01 18:49:43.000000000 +0100
@@ -30,8 +30,14 @@
#include <gecko_sdk/include/nsXPCOM.h>
#include <gecko_sdk/include/nsIDOMEventTarget.h>
#elif BROWSER_IE
+#ifdef WINCE
+// On WinCE we use privateSendUnloadEvent instead of IHTMLWindow3 and
+// attachEvent().
+#include <webvw.h>
+#else
#include <mshtml.h>
#include "gears/base/ie/atl_headers.h"
+#endif
#endif
#include "gears/base/common/base_class.h"
@@ -59,7 +65,11 @@
#if BROWSER_FF
bool Start(nsIDOMEventTarget *event_source);
#elif BROWSER_IE
+#ifdef WINCE
+ bool Start(IPIEHTMLWindow2 *event_source);
+#else
bool Start(IHTMLWindow3 *event_source);
+#endif
#elif BROWSER_NPAPI
bool Start(NPP context, NPObject *event_source);
#endif
@@ -75,7 +85,11 @@
#if BROWSER_FF
nsCOMPtr<nsIDOMEventTarget> event_source_;
#elif BROWSER_IE
+#ifdef WINCE
+ CComPtr<IPIEHTMLWindow2> event_source_;
+#else
CComPtr<IHTMLWindow3> event_source_;
+#endif
#elif BROWSER_NPAPI
NPP event_context_;
ScopedNPVariant event_source_;
====
//depot/googleclient/gears/opensource/gears/base/common/html_event_monitor_iemobile.cc#1
-
c:\GearsWinMo/googleclient/gears/opensource/gears/base/common/html_event_monitor_iemobile.cc
====
# action=add type=text
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++
googleclient/gears/opensource/gears/base/common/html_event_monitor_iemobile.cc
2008-10-01 19:16:41.000000000 +0100
@@ -0,0 +1,70 @@
+// Copyright 2007, 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.
+
+#include "gears/base/common/html_event_monitor.h"
+
+#ifdef WINCE
+#include "gears/base/common/wince_compatibility.h"
+#endif
+
+class HtmlEventMonitorHook : public UnloadEventHandlerInterface {
+ public:
+ HtmlEventMonitorHook(HtmlEventMonitor::HtmlEventCallback function,
+ void *user_param);
+ // UnloadEventHandlerInterface
+ virtual void HandleUnload();
+ private:
+ HtmlEventMonitor::HtmlEventCallback function_;
+ void *user_param_;
+ DISALLOW_EVIL_CONSTRUCTORS(HtmlEventMonitorHook);
+};
+
+HtmlEventMonitorHook::HtmlEventMonitorHook(
+ HtmlEventMonitor::HtmlEventCallback function,
+ void *user_param) : function_(function), user_param_(user_param) {
+}
+
+void HtmlEventMonitorHook::HandleUnload() {
+ function_(user_param_);
+}
+
+//
+// HtmlEventMonitor
+//
+
+bool HtmlEventMonitor::Start(IPIEHTMLWindow2 *event_source) {
+ // attach the event hook to the "manual" source.
+ event_hook_ = new HtmlEventMonitorHook(function_, user_param_);
+ GetUnloadEventSource()->RegisterHandler(event_hook_);
+ // TODO(andreip): attach the hook to the IPIEHTMLWindow2.
+ return true;
+}
+
+void HtmlEventMonitor::Stop() {
+ GetUnloadEventSource()->UnregisterHandler();
+ FreeUnloadEventSource();
+ delete event_hook_;
+ event_hook_ = NULL;
+}
==== //depot/googleclient/gears/opensource/gears/base/common/js_runner_ie.cc#19
- c:\GearsWinMo/googleclient/gears/opensource/gears/base/common/js_runner_ie.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/js_runner_ie.cc
2008-10-01 19:07:30.000000000 +0100
+++ googleclient/gears/opensource/gears/base/common/js_runner_ie.cc
2008-10-01 18:49:13.000000000 +0100
@@ -45,11 +45,7 @@
#include "gears/base/common/base_class.h"
#include "gears/base/common/basictypes.h" // for DISALLOW_EVIL_CONSTRUCTORS
#include "gears/base/common/exception_handler.h"
-#ifdef WINCE
-// WinCE does not use HtmlEventMonitor to monitor page unloading.
-#else
#include "gears/base/common/html_event_monitor.h"
-#endif
#include "gears/base/common/js_runner_utils.h" // For ThrowGlobalErrorImpl()
#ifdef WINCE
#include "gears/base/common/wince_compatibility.h" // For CallWindowOnerror()
@@ -988,20 +984,23 @@
}
bool ListenForUnloadEvent() {
-#ifdef WINCE
- // WinCE does not use HtmlEventMonitor to monitor page unloading.
-#else
// Create an HTML event monitor to send the unload event when the page
// goes away.
unload_monitor_.reset(new HtmlEventMonitor(kEventUnload,
HandleEventUnload, this));
+#ifdef WINCE
+ CComPtr<IPIEHTMLWindow2> event_source;
+ if (FAILED(ActiveXUtils::GetHtmlWindow2(site_, &event_source))) {
+ return false;
+ }
+#else
CComPtr<IHTMLWindow3> event_source;
if (FAILED(ActiveXUtils::GetHtmlWindow3(site_, &event_source))) {
return false;
}
+#endif
unload_monitor_->Start(event_source);
is_unloaded_ = false;
-#endif
return true;
}
@@ -1050,11 +1049,7 @@
}
}
-#ifdef WINCE
- // WinCE does not use HtmlEventMonitor to monitor page unloading.
-#else
scoped_ptr<HtmlEventMonitor> unload_monitor_; // For 'onunload'
notifications
-#endif
CComPtr<IUnknown> site_;
bool is_module_environment_detached_;
bool is_unloaded_;
====
//depot/googleclient/gears/opensource/gears/base/common/wince_compatibility.cc#4
-
c:\GearsWinMo/googleclient/gears/opensource/gears/base/common/wince_compatibility.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/wince_compatibility.cc
2008-10-01 19:07:30.000000000 +0100
+++ googleclient/gears/opensource/gears/base/common/wince_compatibility.cc
2008-10-01 19:16:16.000000000 +0100
@@ -493,4 +493,49 @@
}
#endif
+// Unload monitoring infrastructure
+
+class UnloadEventSource : public UnloadEventSourceInterface {
+ public:
+ UnloadEventSource();
+ // UnloadEventSourceInterface
+ virtual void SendUnload();
+ virtual void RegisterHandler(UnloadEventHandlerInterface *handler);
+ virtual void UnregisterHandler();
+ private:
+ UnloadEventHandlerInterface *handler_;
+ DISALLOW_EVIL_CONSTRUCTORS(UnloadEventSource);
+};
+
+UnloadEventSource *s_unload_event_source_ = NULL;
+
+UnloadEventSource::UnloadEventSource() : handler_(NULL) {
+}
+
+void UnloadEventSource::SendUnload() {
+ if (handler_) handler_->HandleUnload();
+}
+
+void UnloadEventSource::RegisterHandler(UnloadEventHandlerInterface *handler) {
+ assert(handler_ == NULL);
+ handler_ = handler;
+}
+
+void UnloadEventSource::UnregisterHandler() {
+ assert(handler_ != NULL);
+ handler_ = NULL;
+}
+
+UnloadEventSourceInterface* GetUnloadEventSource() {
+ if (!s_unload_event_source_) {
+ s_unload_event_source_ = new UnloadEventSource();
+ }
+ return s_unload_event_source_;
+}
+
+void FreeUnloadEventSource() {
+ delete s_unload_event_source_;
+ s_unload_event_source_ = NULL;
+}
+
#endif // WINCE
====
//depot/googleclient/gears/opensource/gears/base/common/wince_compatibility.h#1
-
c:\GearsWinMo/googleclient/gears/opensource/gears/base/common/wince_compatibility.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/wince_compatibility.h
2008-10-01 19:07:30.000000000 +0100
+++ googleclient/gears/opensource/gears/base/common/wince_compatibility.h
2008-10-01 19:16:00.000000000 +0100
@@ -121,5 +121,29 @@
void CallWindowOnerror(JsRunnerInterface *js_runner,
const std::string16 &message);
+// This is a WinCE specific interface for entities that wish to handle
+// page unload events. Implementors of this interface would register with a
+// concrete implementation of UnloadEventSourceInterface in order to receive
+// the page unload events.
+class UnloadEventHandlerInterface {
+public:
+ virtual ~UnloadEventHandlerInterface() {}
+ virtual void HandleUnload() = 0;
+};
+// This is a WinCE specific interface for entities that are capable of
+// notifying a concrete implementation of UnloadEventHandlerInterface that
+// the page is unloading. Such an entity is the GearsFactoryImpl class which,
+// with its PrivateSendUnloadEvent, can detect unload events.
+class UnloadEventSourceInterface {
+ public:
+ virtual ~UnloadEventSourceInterface() {}
+ virtual void SendUnload() = 0;
+ virtual void RegisterHandler(UnloadEventHandlerInterface *) = 0;
+ virtual void UnregisterHandler() = 0;
+};
+
+UnloadEventSourceInterface* GetUnloadEventSource();
+void FreeUnloadEventSource();
+
#endif // GEARS_BASE_COMMON_WINCE_COMPATIBILITY_H__
#endif // WINCE
==== //depot/googleclient/gears/opensource/gears/factory/factory_impl.cc#17 -
c:\GearsWinMo/googleclient/gears/opensource/gears/factory/factory_impl.cc ====
# action=edit type=text
--- googleclient/gears/opensource/gears/factory/factory_impl.cc 2008-10-01
19:07:31.000000000 +0100
+++ googleclient/gears/opensource/gears/factory/factory_impl.cc 2008-10-01
18:37:04.000000000 +0100
@@ -31,6 +31,9 @@
#include "gears/base/common/base_class.h"
#include "gears/base/common/detect_version_collision.h"
#include "gears/base/common/string16.h"
+#ifdef WINCE
+#include "gears/base/common/wince_compatibility.h"
+#endif
#ifdef DEBUG
#include "gears/blob/fail_blob.h"
#include "gears/blob/blob.h"
@@ -86,6 +89,8 @@
#ifdef WINCE
RegisterMethod("privateSetGlobalObject",
&GearsFactoryImpl::PrivateSetGlobalObject);
+ RegisterMethod("privateSendUnloadEvent",
+ &GearsFactoryImpl::PrivateSendUnloadEvent);
#endif
}
@@ -311,6 +316,10 @@
// a valid call, rather than throw a "no such method" exception, and hence
// we provide that empty method here in GearsFactoryImpl.
}
+
+void GearsFactoryImpl::PrivateSendUnloadEvent(JsCallContext *context) {
+ GetUnloadEventSource()->SendUnload();
+}
#endif
// TODO(cprince): See if we can use Suspend/Resume with the opt-in dialog too,
==== //depot/googleclient/gears/opensource/gears/factory/factory_impl.h#4 -
c:\GearsWinMo/googleclient/gears/opensource/gears/factory/factory_impl.h ====
# action=edit type=text
--- googleclient/gears/opensource/gears/factory/factory_impl.h 2008-10-01
19:07:31.000000000 +0100
+++ googleclient/gears/opensource/gears/factory/factory_impl.h 2008-10-01
15:50:17.000000000 +0100
@@ -66,6 +66,9 @@
// IN: -
// OUT: -
void PrivateSetGlobalObject(JsCallContext *context);
+ // IN: -
+ // OUT: -
+ void PrivateSendUnloadEvent(JsCallContext *context);
#endif
// Non-scriptable methods
