IE, Chrome, Firefox plugins: gracefully disconnect when server connection drops.
1) Plugins fails gracefully in the face of a disconnect, returning undefined instead of making lots of noise.
2) Plugins invokes hosted.html's __gwt_disconnected() method the first time the session is detected as dropped. This glasspanels the app.
Review by: jat
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@7129 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/plugins/common/HostChannel.cpp b/plugins/common/HostChannel.cpp
index 90552a4..bff1b21 100644
--- a/plugins/common/HostChannel.cpp
+++ b/plugins/common/HostChannel.cpp
@@ -68,6 +68,7 @@
bool HostChannel::init(SessionHandler* handler, int minProtoVers,
int maxProtoVers, const std::string& hostedHtmlVers) {
+ this->handler = handler;
Debug::log(Debug::Debugging)
<< " negotiating versions - we support protocol " << minProtoVers
<< " through " << maxProtoVers << ", hostedHtmlVersion=" << hostedHtmlVers
diff --git a/plugins/common/HostChannel.h b/plugins/common/HostChannel.h
index f104d63..3c5c5e3 100644
--- a/plugins/common/HostChannel.h
+++ b/plugins/common/HostChannel.h
@@ -33,6 +33,7 @@
class HostChannel {
Socket sock;
static ByteOrder byteOrder;
+ SessionHandler* handler;
public:
~HostChannel() {
@@ -95,10 +96,12 @@
bool readByte(char& data) {
if (!isConnected()) {
+ handler->disconnectDetected();
return false;
}
int c = sock.readByte();
if (c < 0) {
+ handler->disconnectDetected();
return false;
}
data = static_cast<char>(c);
@@ -107,9 +110,14 @@
bool sendByte(const char data) {
if (!isConnected()) {
+ handler->disconnectDetected();
return false;
}
- return sock.writeByte(data);
+ if (!sock.writeByte(data)) {
+ handler->disconnectDetected();
+ return false;
+ }
+ return true;
}
bool readStringLength(uint32_t& data);
@@ -133,7 +141,15 @@
}
bool flush() {
- return sock.flush();
+ if (!sock.isConnected()) {
+ handler->disconnectDetected();
+ return false;
+ }
+ if (!sock.flush()) {
+ handler->disconnectDetected();
+ return false;
+ }
+ return true;
}
ReturnMessage* reactToMessagesWhileWaitingForReturn(SessionHandler* handler) {
diff --git a/plugins/common/SessionHandler.h b/plugins/common/SessionHandler.h
index 2cb011f..aebd00d 100644
--- a/plugins/common/SessionHandler.h
+++ b/plugins/common/SessionHandler.h
@@ -49,6 +49,24 @@
SetProperty = SPECIAL_SET_PROPERTY
};
protected:
+ SessionHandler(): alreadyDisconnected(false) {
+ }
+
+ /**
+ * Called by the server socket when it cannot read, write, or flush.
+ */
+ void disconnectDetected() {
+ if (!alreadyDisconnected) {
+ alreadyDisconnected = true;
+ disconnectDetectedImpl();
+ }
+ }
+
+ /**
+ * Implementors should invoke __gwt_disconnected() in the hosted.html window
+ * to "glass" the screen with a disconnect message.
+ */
+ virtual void disconnectDetectedImpl() = 0;
/**
* Report a fatal error -- the channel will be closed after this method
@@ -84,6 +102,9 @@
virtual void sendFreeValues(HostChannel& channel) = 0;
virtual ~SessionHandler() {}
+
+private:
+ bool alreadyDisconnected;
};
#endif
diff --git a/plugins/ie/oophm/oophm/IESessionHandler.cpp b/plugins/ie/oophm/oophm/IESessionHandler.cpp
index 5491496..be2ddf8 100644
--- a/plugins/ie/oophm/oophm/IESessionHandler.cpp
+++ b/plugins/ie/oophm/oophm/IESessionHandler.cpp
@@ -45,6 +45,22 @@
channel->disconnectFromHost();
}
+void IESessionHandler::disconnectDetectedImpl() {
+ DISPID dispId;
+ LPOLESTR gwtDisconnectedName = L"__gwt_disconnected";
+ if (!SUCCEEDED(getWindow()->GetIDsOfNames(IID_NULL, &gwtDisconnectedName, 1,
+ LOCALE_SYSTEM_DEFAULT, &dispId))) {
+ Debug::log(Debug::Error) << "Unable to get dispId for __gwt_disconnected" << Debug::flush;
+ return;
+ }
+
+ DISPPARAMS dispParams = {NULL, NULL, 0, 0};
+ CComPtr<IDispatchEx> dispEx;
+ getWindow()->QueryInterface(&dispEx);
+ dispEx->InvokeEx(dispId, LOCALE_SYSTEM_DEFAULT, DISPATCH_METHOD,
+ &dispParams, NULL, NULL, NULL);
+}
+
void IESessionHandler::fatalError(HostChannel& channel,
const std::string& message) {
// TODO: better way of reporting error?
diff --git a/plugins/ie/oophm/oophm/IESessionHandler.h b/plugins/ie/oophm/oophm/IESessionHandler.h
index 2db361a..2675810 100644
--- a/plugins/ie/oophm/oophm/IESessionHandler.h
+++ b/plugins/ie/oophm/oophm/IESessionHandler.h
@@ -36,6 +36,7 @@
virtual void makeValueRef(_variant_t& value, const Value& in);
protected:
+ virtual void disconnectDetectedImpl();
virtual void fatalError(HostChannel& channel, const std::string& messsage);
virtual void freeValue(HostChannel& channel, int idCount, const int* ids);
virtual void loadJsni(HostChannel& channel, const std::string& js);
diff --git a/plugins/ie/oophm/oophm/JavaObject.cpp b/plugins/ie/oophm/oophm/JavaObject.cpp
index cba4d71..2db7977 100644
--- a/plugins/ie/oophm/oophm/JavaObject.cpp
+++ b/plugins/ie/oophm/oophm/JavaObject.cpp
@@ -152,32 +152,30 @@
javaDispatchId.setInt(0);
}
+ bool isException = false;
+ Value returnValue;
if (!InvokeMessage::send(*channel, thisRef, javaDispatchId.getInt(), numArgs, args.get())) {
Debug::log(Debug::Error) << "Unable to send method invocation" << Debug::flush;
- return E_FAIL;
- }
-
- scoped_ptr<ReturnMessage> m(channel->reactToMessagesWhileWaitingForReturn(
+ } else {
+ scoped_ptr<ReturnMessage> m(channel->reactToMessagesWhileWaitingForReturn(
sessionData->getSessionHandler()));
- if (!m.get()) {
- Debug::log(Debug::Error) << "Did not receive ReturnMessage" << Debug::flush;
- if (pvarResult) {
- VariantClear(pvarResult);
+ if (!m.get()) {
+ Debug::log(Debug::Error) << "Did not receive ReturnMessage" << Debug::flush;
+ } else {
+ if (dispidMember == DISPID_TOSTRING) {
+ // raw toString();
+ if (pvarResult) {
+ // This will be NULL when the caller doesn't care about the return value
+ _variant_t returnVariant;
+ sessionData->makeValueRef(returnVariant, m->getReturnValue());
+ *pvarResult = returnVariant.Detach();
+ }
+ return m->isException() ? E_FAIL : S_OK;
+ }
+ isException = m->isException();
+ returnValue = m->getReturnValue();
}
- // XXX better error handling
- return E_FAIL;
- }
-
- if (dispidMember == DISPID_TOSTRING) {
- // raw toString();
- if (pvarResult) {
- // This will be NULL when the caller doesn't care about the return value
- _variant_t returnVariant;
- sessionData->makeValueRef(returnVariant, m->getReturnValue());
- *pvarResult = returnVariant.Detach();
- }
- return m->isException() ? E_FAIL : S_OK;
}
DISPID dispId;
@@ -191,8 +189,8 @@
// Call __gwt_makeResult(isException, returnValue)
scoped_array<_variant_t> varArgs(new _variant_t[2]);
// Args go backwards.
- varArgs[1] = (m->isException() ? 1 : 0);
- sessionData->makeValueRef(varArgs[0], m->getReturnValue());
+ varArgs[1] = (isException ? 1 : 0);
+ sessionData->makeValueRef(varArgs[0], returnValue);
DISPPARAMS dispParams = {varArgs.get(), NULL, 2, 0};
CComPtr<IDispatchEx> dispEx;
sessionData->getWindow()->QueryInterface(&dispEx);
diff --git a/plugins/npapi/LocalObjectTable.h b/plugins/npapi/LocalObjectTable.h
index 7d95509..1fa69c2 100644
--- a/plugins/npapi/LocalObjectTable.h
+++ b/plugins/npapi/LocalObjectTable.h
@@ -38,8 +38,7 @@
}
public:
- LocalObjectTable() {
- dontFree = false;
+ LocalObjectTable(): nextId(0), dontFree(false) {
}
virtual ~LocalObjectTable();
@@ -68,11 +67,11 @@
Debug::log(Debug::Error) << "Freeing freed object slot " << id << Debug::flush;
return;
}
- setFree(id);
if (!dontFree) {
NPObject* obj = it->second;
NPN_ReleaseObject(obj);
}
+ setFree(id);
}
void freeAll() {
diff --git a/plugins/npapi/ScriptableInstance.cpp b/plugins/npapi/ScriptableInstance.cpp
index 6149ffc..273fd8a 100644
--- a/plugins/npapi/ScriptableInstance.cpp
+++ b/plugins/npapi/ScriptableInstance.cpp
@@ -63,6 +63,7 @@
connectedID(NPN_GetStringIdentifier("connected")),
statsID(NPN_GetStringIdentifier("stats")),
gwtId(NPN_GetStringIdentifier("__gwt_ObjectId")),
+ jsDisconnectedID(NPN_GetStringIdentifier("__gwt_disconnected")),
jsInvokeID(NPN_GetStringIdentifier("__gwt_jsInvoke")),
jsResultID(NPN_GetStringIdentifier("__gwt_makeResult")),
jsTearOffID(NPN_GetStringIdentifier("__gwt_makeTearOff")),
@@ -542,24 +543,27 @@
for (unsigned i = 0; i < numArgs; ++i) {
vargs[i] = NPVariantProxy::getAsValue(args[i], *this);
}
+ bool isException = false;
+ Value returnValue;
if (!InvokeMessage::send(*_channel, javaThis, dispId, numArgs, vargs.get())) {
Debug::log(Debug::Error) << "JavaObject_invoke: failed to send invoke message" << Debug::flush;
- // TODO(jat): returning false here spams the browser console
- return true;
- }
- Debug::log(Debug::Debugging) << " return from invoke" << Debug::flush;
- scoped_ptr<ReturnMessage> retMsg(_channel->reactToMessagesWhileWaitingForReturn(this));
- if (!retMsg.get()) {
- Debug::log(Debug::Error) << "JavaObject_invoke: failed to get return value" << Debug::flush;
- return false;
- }
- if (isRawToString) {
- // toString() needs the raw value
- NPVariantProxy::assignFrom(*this, *result, retMsg->getReturnValue());
- return !retMsg->isException();
+ } else {
+ Debug::log(Debug::Debugging) << " return from invoke" << Debug::flush;
+ scoped_ptr<ReturnMessage> retMsg(_channel->reactToMessagesWhileWaitingForReturn(this));
+ if (!retMsg.get()) {
+ Debug::log(Debug::Error) << "JavaObject_invoke: failed to get return value" << Debug::flush;
+ } else {
+ if (isRawToString) {
+ // toString() needs the raw value
+ NPVariantProxy::assignFrom(*this, *result, retMsg->getReturnValue());
+ return !retMsg->isException();
+ }
+ isException = retMsg->isException();
+ returnValue = retMsg->getReturnValue();
+ }
}
// Wrap the result
- return makeResult(retMsg->isException(), retMsg->getReturnValue(), result);
+ return makeResult(isException, returnValue, result);
}
bool ScriptableInstance::JavaObject_getProperty(int objectId, int dispId,
@@ -651,6 +655,11 @@
javaObjectsToFree.insert(objectId);
}
+void ScriptableInstance::disconnectDetectedImpl() {
+ NPVariantWrapper result(*this);
+ NPN_Invoke(getNPP(), window, jsDisconnectedID, 0, 0, result.addressForReturn());
+}
+
void ScriptableInstance::sendFreeValues(HostChannel& channel) {
unsigned n = javaObjectsToFree.size();
if (n) {
diff --git a/plugins/npapi/ScriptableInstance.h b/plugins/npapi/ScriptableInstance.h
index cdce6f3..2d03a67 100644
--- a/plugins/npapi/ScriptableInstance.h
+++ b/plugins/npapi/ScriptableInstance.h
@@ -72,6 +72,10 @@
void destroyJavaWrapper(JavaObject*);
static const uint32_t VERSION = 1;
+
+protected:
+ virtual void disconnectDetectedImpl();
+
private:
// Map of object ID to JavaObject
hash_map<int, JavaObject*> javaObjects;
@@ -94,6 +98,7 @@
const NPIdentifier statsID;
const NPIdentifier gwtId;
+ const NPIdentifier jsDisconnectedID;
const NPIdentifier jsInvokeID;
const NPIdentifier jsResultID;
const NPIdentifier jsTearOffID;
diff --git a/plugins/xpcom/FFSessionHandler.cpp b/plugins/xpcom/FFSessionHandler.cpp
index a2b5cc4..b7ad7ff 100755
--- a/plugins/xpcom/FFSessionHandler.cpp
+++ b/plugins/xpcom/FFSessionHandler.cpp
@@ -128,6 +128,26 @@
}
}
+void FFSessionHandler::disconnectDetectedImpl() {
+ JSContext* ctx = getJSContext();
+ if (!ctx) {
+ return;
+ }
+
+ Debug::log(Debug::Debugging) << "Getting function \"__gwt_disconnected\""
+ << Debug::flush;
+
+ jsval funcVal;
+ if (!JS_GetProperty(ctx, global, "__gwt_disconnected", &funcVal)
+ || funcVal == JSVAL_VOID) {
+ Debug::log(Debug::Error) << "Could not get function \"__gwt_disconnected\""
+ << Debug::flush;
+ return;
+ }
+ jsval rval;
+ JS_CallFunctionValue(ctx, global, funcVal, 0, 0, &rval);
+}
+
void FFSessionHandler::freeValue(HostChannel& channel, int idCount, const int* ids) {
Debug::DebugStream& dbg = Debug::log(Debug::Spam)
<< "FFSessionHandler::freeValue [ ";
diff --git a/plugins/xpcom/FFSessionHandler.h b/plugins/xpcom/FFSessionHandler.h
index f7fec2b..1da4288 100755
--- a/plugins/xpcom/FFSessionHandler.h
+++ b/plugins/xpcom/FFSessionHandler.h
@@ -41,6 +41,7 @@
void disconnect();
protected:
+ virtual void disconnectDetectedImpl();
virtual void freeValue(HostChannel& channel, int idCount, const int* ids);
virtual void loadJsni(HostChannel& channel, const std::string& js);
virtual bool invoke(HostChannel& channel, const Value& thisObj, const std::string& methodName,
diff --git a/plugins/xpcom/JavaObject.cpp b/plugins/xpcom/JavaObject.cpp
index 638094f..ca0104b 100644
--- a/plugins/xpcom/JavaObject.cpp
+++ b/plugins/xpcom/JavaObject.cpp
@@ -359,17 +359,21 @@
for (int i = 0; i < numArgs; ++i) {
data->makeValueFromJsval(args[i], ctx, jsargs[i]);
}
+
+ bool isException = false;
+ Value returnValue;
if (!InvokeMessage::send(*channel, javaThis, dispId, numArgs, args.get())) {
Debug::log(Debug::Debugging) << "JavaObject::call failed to send invoke message" << Debug::flush;
- return false;
+ } else {
+ Debug::log(Debug::Spam) << " return from invoke" << Debug::flush;
+ scoped_ptr<ReturnMessage> retMsg(channel->reactToMessagesWhileWaitingForReturn(handler));
+ if (!retMsg.get()) {
+ Debug::log(Debug::Debugging) << "JavaObject::call failed to get return value" << Debug::flush;
+ } else {
+ isException = retMsg->isException();
+ returnValue = retMsg->getReturnValue();
+ }
}
- Debug::log(Debug::Spam) << " return from invoke" << Debug::flush;
- scoped_ptr<ReturnMessage> retMsg(channel->reactToMessagesWhileWaitingForReturn(handler));
- if (!retMsg.get()) {
- Debug::log(Debug::Debugging) << "JavaObject::call failed to get return value" << Debug::flush;
- return false;
- }
- Value returnValue = retMsg->getReturnValue();
// Since we can set exceptions normally, we always return false to the
// wrapper function and set the exception ourselves if one occurs.
// TODO: cleanup exception case
@@ -379,7 +383,7 @@
jsval retJsVal;
Debug::log(Debug::Spam) << " result is " << returnValue << Debug::flush;
data->makeJsvalFromValue(retJsVal, ctx, returnValue);
- if (retMsg->isException()) {
+ if (isException) {
JS_SetPendingException(ctx, retJsVal);
return false;
}
