Browse Source

fix safari crash

David Rose 16 years ago
parent
commit
7e565c2a49

+ 12 - 0
direct/src/plugin/p3dDownload.I

@@ -73,6 +73,18 @@ get_download_success() const {
   return _status == P3D_RC_done;
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: P3DDownload::get_download_terminated
+//       Access: Public
+//  Description: Returns true if the download has failed because the
+//               instance is about to be shut down, or false if it
+//               hasn't failed, or failed for some other reason.
+////////////////////////////////////////////////////////////////////
+inline bool P3DDownload::
+get_download_terminated() const {
+  return _status == P3D_RC_shutdown;
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: P3DDownload::set_download_id
 //       Access: Public

+ 1 - 0
direct/src/plugin/p3dDownload.h

@@ -40,6 +40,7 @@ public:
   inline double get_download_progress() const;
   inline bool get_download_finished() const;
   inline bool get_download_success() const;
+  inline bool get_download_terminated() const;
   inline size_t get_total_data() const;
 
   void cancel();

+ 18 - 1
direct/src/plugin/p3dPackage.cxx

@@ -712,7 +712,7 @@ got_desc_file(TiXmlDocument *doc, bool freshly_downloaded) {
 ////////////////////////////////////////////////////////////////////
 //     Function: P3DPackage::clear_install_plans
 //       Access: Private
-//  Description:a Empties _install_plans cleanly.
+//  Description: Empties _install_plans cleanly.
 ////////////////////////////////////////////////////////////////////
 void P3DPackage::
 clear_install_plans() {
@@ -883,6 +883,12 @@ follow_install_plans(bool download_finished) {
         plan_failed = true;
         break;
 
+      case IT_terminate:
+        // All plans have failed.
+        _install_plans.clear();
+        report_done(false);
+        return;
+
       case IT_continue:
         // A callback hook has been attached; we'll come back later.
         return;
@@ -1213,6 +1219,13 @@ download_finished(bool success) {
   close_file();
 
   if (!success) {
+    if (get_download_terminated()) {
+      // Short-circuit the exit.
+      _try_urls.clear();
+      resume_download_finished(false);
+      return;
+    }
+
     // Maybe it failed because our contents.xml file is out-of-date.
     // Go try to freshen it.
     bool is_contents_file = (_dtype == DT_contents_file || _dtype == DT_redownload_contents_file);
@@ -1352,6 +1365,10 @@ do_step(bool download_finished) {
   if (_download->get_download_success()) {
     // The Download object has already validated the hash.
     return IT_step_complete;
+  } else if (_download->get_download_terminated()) {
+    // The download was interrupted because we're shutting down.
+    // Don't try any other plans.
+    return IT_terminate;
   } else {
     // The Download object has already tried all of the mirrors, and
     // they all failed.

+ 1 - 0
direct/src/plugin/p3dPackage.h

@@ -112,6 +112,7 @@ private:
     IT_step_complete,
     IT_step_failed,
     IT_continue,
+    IT_terminate,
   };
 
   class InstallStep {

+ 6 - 0
direct/src/plugin/p3d_plugin.h

@@ -831,6 +831,12 @@ typedef enum {
      There may or may not be data associated with this error as well.
      However, no more data will be delivered after this call. */
   P3D_RC_http_error,
+
+  /* The download was interrupted because we're about to shut down the
+     instance.  The instance should not attempt to restart a new
+     download in response to this. */
+  P3D_RC_shutdown
+
 } P3D_result_code;
 
 /* This function is used by the host to handle a get_url request,

+ 72 - 15
direct/src/plugin_npapi/ppInstance.cxx

@@ -84,6 +84,8 @@ PPInstance::
     _p3d_inst = NULL;
   }
 
+  assert(_streams.empty());
+
   if (_script_object != NULL) {
     browser->releaseobject(_script_object);
   }
@@ -187,6 +189,8 @@ set_window(NPWindow *window) {
 ////////////////////////////////////////////////////////////////////
 NPError PPInstance::
 new_stream(NPMIMEType type, NPStream *stream, bool seekable, uint16 *stype) {
+  assert(find(_streams.begin(), _streams.end(), stream) == _streams.end());
+
   if (stream->notifyData == NULL) {
     // This is an unsolicited stream.  Assume the first unsolicited
     // stream we receive is the instance data; any other unsolicited
@@ -198,6 +202,7 @@ new_stream(NPMIMEType type, NPStream *stream, bool seekable, uint16 *stype) {
       stream->notifyData = new PPDownloadRequest(PPDownloadRequest::RT_instance_data);
       
       *stype = NP_NORMAL;
+      _streams.push_back(stream);
       return NPERR_NO_ERROR;
     }
 
@@ -211,18 +216,21 @@ new_stream(NPMIMEType type, NPStream *stream, bool seekable, uint16 *stype) {
     // This is the initial contents.xml file.  We'll just download
     // this directly to a file, since it is small and this is easy.
     *stype = NP_ASFILEONLY;
+    _streams.push_back(stream);
     return NPERR_NO_ERROR;
 
   case PPDownloadRequest::RT_core_dll:
     // This is the core API DLL (or dylib or whatever).  We want to
     // download this to file for convenience.
     *stype = NP_ASFILEONLY;
+    _streams.push_back(stream);
     return NPERR_NO_ERROR;
 
   case PPDownloadRequest::RT_user:
     // This is a request from the plugin.  We'll receive this as a
     // stream.
     *stype = NP_NORMAL;
+    _streams.push_back(stream);
     return NPERR_NO_ERROR;
 
   default:
@@ -233,6 +241,29 @@ new_stream(NPMIMEType type, NPStream *stream, bool seekable, uint16 *stype) {
   return NPERR_GENERIC_ERROR;
 }
 
+
+////////////////////////////////////////////////////////////////////
+//     Function: PPInstance::stop_outstanding_streams
+//       Access: Public
+//  Description: Stops any download streams that are currently active
+//               on the instance.  It is necessary to call this
+//               explicitly before destroying the instance, at least
+//               for Safari.
+////////////////////////////////////////////////////////////////////
+void PPInstance::
+stop_outstanding_streams() {
+  Streams::iterator si;
+  Streams streams;
+  streams.swap(_streams);
+  for (si = streams.begin(); si != streams.end(); ++si) {
+    NPStream *stream = (*si);
+    nout << "Stopping stream " << stream->url << "\n";
+    browser->destroystream(_npp_instance, stream, NPRES_USER_BREAK);
+  }
+
+  assert(_streams.empty());
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: PPInstance::write_ready
 //       Access: Public
@@ -310,6 +341,13 @@ write_stream(NPStream *stream, int offset, int len, void *buffer) {
 ////////////////////////////////////////////////////////////////////
 NPError PPInstance::
 destroy_stream(NPStream *stream, NPReason reason) {
+  Streams::iterator si = find(_streams.begin(), _streams.end(), stream);
+  if (si == _streams.end()) {
+    nout << "Got destroy_stream for unknown stream\n";
+  } else {
+    _streams.erase(si);
+  }
+
   if (stream->notifyData == NULL) {
     nout << "Unexpected destroy_stream on " << stream->url << "\n";
     return NPERR_NO_ERROR;
@@ -321,7 +359,11 @@ destroy_stream(NPStream *stream, NPReason reason) {
     {
       P3D_result_code result_code = P3D_RC_done;
       if (reason != NPRES_DONE) {
-        result_code = P3D_RC_generic_error;
+        if (reason == NPRES_USER_BREAK) {
+          result_code = P3D_RC_shutdown;
+        } else {
+          result_code = P3D_RC_generic_error;
+        }
       }
       assert(!req->_notified_done);
       P3D_instance_feed_url_stream(_p3d_inst, req->_user_id,
@@ -330,6 +372,12 @@ destroy_stream(NPStream *stream, NPReason reason) {
     }
     break;
 
+  case PPDownloadRequest::RT_core_dll:
+  case PPDownloadRequest::RT_contents_file:
+    // These are received as a full-file only, so we don't care about
+    // the destroy_stream notification.
+    break;
+
   default:
     nout << "Unexpected destroy_stream on " << stream->url << "\n";
     break;
@@ -372,13 +420,17 @@ url_notify(const char *url, NPReason reason, void *notifyData) {
     if (reason != NPRES_DONE) {
       nout << "Failure downloading " << url << "\n";
 
-      // Couldn't download a fresh contents.xml for some reason.  If
-      // there's an outstanding contents.xml file on disk, try to load
-      // that one as a fallback.
-      string contents_filename = _root_dir + "/contents.xml";
-      if (!read_contents_file(contents_filename)) {
-        nout << "Unable to read contents file " << contents_filename << "\n";
-        // TODO: fail
+      if (reason == NPRES_USER_BREAK) {
+        nout << "Failure due to user break\n";
+      } else {
+        // Couldn't download a fresh contents.xml for some reason.  If
+        // there's an outstanding contents.xml file on disk, try to
+        // load that one as a fallback.
+        string contents_filename = _root_dir + "/contents.xml";
+        if (!read_contents_file(contents_filename)) {
+          nout << "Unable to read contents file " << contents_filename << "\n";
+          // TODO: fail
+        }
       }
     }
     break;
@@ -386,13 +438,18 @@ url_notify(const char *url, NPReason reason, void *notifyData) {
   case PPDownloadRequest::RT_core_dll:
     if (reason != NPRES_DONE) {
       nout << "Failure downloading " << url << "\n";
-      // Couldn't download from this mirror.  Try the next one.
-      if (!_core_urls.empty()) {
-        string url = _core_urls.back();
-        _core_urls.pop_back();
-        
-        PPDownloadRequest *req2 = new PPDownloadRequest(PPDownloadRequest::RT_core_dll);
-        start_download(url, req2);
+
+      if (reason == NPRES_USER_BREAK) {
+        nout << "Failure due to user break\n";
+      } else {
+        // Couldn't download from this mirror.  Try the next one.
+        if (!_core_urls.empty()) {
+          string url = _core_urls.back();
+          _core_urls.pop_back();
+          
+          PPDownloadRequest *req2 = new PPDownloadRequest(PPDownloadRequest::RT_core_dll);
+          start_download(url, req2);
+        }
       }
     }
     break;

+ 7 - 0
direct/src/plugin_npapi/ppInstance.h

@@ -46,6 +46,7 @@ public:
   void set_window(NPWindow *window);
   NPError new_stream(NPMIMEType type, NPStream *stream, 
                      bool seekable, uint16 *stype);
+  void stop_outstanding_streams();
 
   int32 write_ready(NPStream *stream);
   int write_stream(NPStream *stream, int offset, int len, void *buffer);
@@ -116,6 +117,12 @@ private:
 
   bool _got_instance_url;
   string _instance_url;
+ 
+  // We need to keep a list of the NPStream objects that the instance
+  // owns, because Safari (at least) won't automatically delete all of
+  // the outstanding streams when the instance is destroyed.
+  typedef vector<NPStream *> Streams;
+  Streams _streams;
 
   // This class is used for feeding local files (accessed via a
   // "file://" url) into the core API.

+ 13 - 5
direct/src/plugin_npapi/startup.cxx

@@ -243,11 +243,12 @@ NP_Shutdown(void) {
 NPError 
 NPP_New(NPMIMEType pluginType, NPP instance, uint16 mode, 
         int16 argc, char *argn[], char *argv[], NPSavedData *saved) {
-  nout << "new instance\n";
+  nout << "new instance " << instance << "\n";
 
   PPInstance *inst = new PPInstance(pluginType, instance, mode,
                                     argc, argn, argv, saved);
   instance->pdata = inst;
+  nout << "new instance->pdata = " << inst << "\n";
 
   // To experiment with a "windowless" plugin, which really means we
   // create our own window without an intervening window, try this.
@@ -267,10 +268,15 @@ NPP_New(NPMIMEType pluginType, NPP instance, uint16 mode,
 ////////////////////////////////////////////////////////////////////
 NPError
 NPP_Destroy(NPP instance, NPSavedData **save) {
-  nout << "destroy instance " << instance << "\n";
+  nout << "destroy instance " << instance << ", " 
+       << (PPInstance *)instance->pdata << "\n";
   nout << "save = " << (void *)save << "\n";
   //  (*save) = NULL;
-  delete (PPInstance *)(instance->pdata);
+  PPInstance *inst = (PPInstance *)(instance->pdata);
+  assert(inst != NULL);
+  inst->stop_outstanding_streams();
+
+  delete inst;
   instance->pdata = NULL;
 
   return NPERR_NO_ERROR;
@@ -330,7 +336,8 @@ NPP_DestroyStream(NPP instance, NPStream *stream, NPReason reason) {
        << ", " << stream->end 
        << ", notifyData = " << stream->notifyData
        << ", reason = " << reason
-       << "\n";
+       << ", for " << instance 
+       << ", " << (PPInstance *)(instance->pdata) << "\n";
 
   PPInstance::generic_browser_call();
   PPInstance *inst = (PPInstance *)(instance->pdata);
@@ -346,6 +353,7 @@ NPP_DestroyStream(NPP instance, NPStream *stream, NPReason reason) {
 ////////////////////////////////////////////////////////////////////
 int32
 NPP_WriteReady(NPP instance, NPStream *stream) {
+  //  nout << "WriteReady " << stream->url << " for " << instance << ", " << (PPInstance *)(instance->pdata) << "\n";
   PPInstance::generic_browser_call();
   PPInstance *inst = (PPInstance *)(instance->pdata);
   assert(inst != NULL);
@@ -362,7 +370,7 @@ NPP_WriteReady(NPP instance, NPStream *stream) {
 int32
 NPP_Write(NPP instance, NPStream *stream, int32 offset, 
           int32 len, void *buffer) {
-  //  nout << "Write " << stream->url << ", " << len << "\n";
+  //  nout << "Write " << stream->url << ", " << len << " for " << instance << ", " << (PPInstance *)(instance->pdata) << "\n";
   PPInstance::generic_browser_call();
   PPInstance *inst = (PPInstance *)(instance->pdata);
   assert(inst != NULL);