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; }