Make the rate limiting of Native Client Plugin progress events more robust.

ppapi/native_client/src/trusted/plugin/plugin.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifdef _MSC_VER
 // Do not warn about use of std::copy with raw pointers.
 #pragma warning(disable : 4996)
 #endif
 
 #include "native_client/src/trusted/plugin/plugin.h"
@@ skipping 967 matching lines @@
       nexe_error_reported_(false),
       wrapper_factory_(NULL),
       last_error_string_(""),
       ppapi_proxy_(NULL),
       enable_dev_interfaces_(false),
       replayDidChangeView(false),
       replayHandleDocumentLoad(false),
       init_time_(0),
       ready_time_(0),
       nexe_size_(0),
-      last_event_bytes_received_(0) {
+      time_of_last_progress_event_(0) {
   PLUGIN_PRINTF(("Plugin::Plugin (this=%p, pp_instance=%"
                  NACL_PRId32")\n", static_cast<void*>(this), pp_instance));
   NaClSrpcModuleInit();
   nexe_downloader_.Initialize(this);
   pnacl_.Initialize(this);
   callback_factory_.Initialize(this);
 }
 
 
 Plugin::~Plugin() {
@@ skipping 837 matching lines @@
 void Plugin::UpdateDownloadProgress(
     PP_Instance pp_instance,
     PP_Resource pp_resource,
     int64_t /*bytes_sent*/,
     int64_t /*total_bytes_to_be_sent*/,
     int64_t bytes_received,
     int64_t total_bytes_to_be_received) {
   Instance* instance = pp::Module::Get()->InstanceForPPInstance(pp_instance);
   if (instance != NULL) {
     Plugin* plugin = static_cast<Plugin*>(instance);
-    int64_t progress = bytes_received - plugin->last_event_bytes_received_;
-    const int64_t kProgressThreshold = 1 << 17;  // 128K bytes per event
-    if (progress > kProgressThreshold) {
+    // Rate limit progress events to a maximum of 100 per second.
+    int64_t time = NaClGetTimeOfDayMicroseconds();
+    int64_t elapsed = time - plugin->time_of_last_progress_event_;
+    const int64_t kTenMilliseconds = 10000;

[polina, 2011/11/23 08:23:59]

perhaps put this at the top of the file?

[bbudge, 2011/11/23 14:09:37]

Since it's only used once, I kind of like it right here. For readability.
On 2011/11/23 08:23:59, polina wrote:

+    if (elapsed > kTenMilliseconds) {
+      plugin->time_of_last_progress_event_ = time;
+
+      // Find the URL loader that sent this notification.
+      const FileDownloader* file_downloader =
+          plugin->FindFileDownloader(pp_resource);
+      // If not a streamed file, it must be the .nexe loader.
+      if (file_downloader == NULL)
+        file_downloader = &plugin->nexe_downloader_;
+      nacl::string url = file_downloader->url_to_open();
       LengthComputable length_computable = (total_bytes_to_be_received >= 0) ?
           LENGTH_IS_COMPUTABLE : LENGTH_IS_NOT_COMPUTABLE;
-      // Get the URL for the URL loader that sent this notification.
-      const FileDownloader* file_downloader =
-          plugin->FindFileDownloader(pp_resource);
-      nacl::string url = (file_downloader != NULL) ?
-          file_downloader->url_to_open() : NACL_NO_URL;
 
       plugin->EnqueueProgressEvent(kProgressEventProgress,
                                    url,
                                    length_computable,
                                    bytes_received,
                                    total_bytes_to_be_received);

[polina, 2011/11/23 08:23:59]

do the users know that they only get some of these events and that if they some
up bytes_received from each one, they might not add up to
total_bytes_to_be_received?

[bbudge, 2011/11/23 14:09:37]

They will always get a 'loadend' event with the received == total.
On 2011/11/23 08:23:59, polina wrote:

-      plugin->last_event_bytes_received_ = bytes_received;
     }
   }
 }
 
 const FileDownloader* Plugin::FindFileDownloader(
     PP_Resource url_loader) const {
   const FileDownloader* file_downloader = NULL;
   if (url_loader == nexe_downloader_.url_loader()) {
     file_downloader = &nexe_downloader_;
   } else {
@@ skipping 141 matching lines @@
   std::string scheme = canonicalized.AsString().substr(comps.scheme.begin,
                                                        comps.scheme.len);
   if (scheme == kChromeExtensionUriScheme)
     return SCHEME_CHROME_EXTENSION;
   if (scheme == kDataUriScheme)
     return SCHEME_DATA;
   return SCHEME_OTHER;
 }
 
 }  // namespace plugin