Move command line processing out of coordinator

ppapi/native_client/src/trusted/plugin/pnacl_resources.h

 // 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.
 
 #ifndef NATIVE_CLIENT_SRC_TRUSTED_PLUGIN_PNACL_RESOURCES_H_
 #define NATIVE_CLIENT_SRC_TRUSTED_PLUGIN_PNACL_RESOURCES_H_
 
 #include <map>
 #include <vector>
 
 #include "native_client/src/include/nacl_macros.h"
 #include "native_client/src/include/nacl_string.h"
 #include "native_client/src/shared/srpc/nacl_srpc.h"
 #include "native_client/src/trusted/desc/nacl_desc_wrapper.h"
 #include "native_client/src/trusted/plugin/delayed_callback.h"
 #include "native_client/src/trusted/plugin/plugin_error.h"
 
 #include "ppapi/cpp/completion_callback.h"
 
 namespace plugin {
 
 class Plugin;
 class PnaclCoordinator;
 
 class PnaclResources {

[robertm, 2011/12/13 17:06:39]

pre-existing but since this is a central class: add class comment

the protocol is actually not quite clear to me:
is this class single use or multiple use?
single use would be easier to understand. 

[sehr (please use chromium), 2011/12/13 20:05:04]

As this CL removes most of the need for the PnaclResources class, I have
drastically simplified its features and added more documentation.

  public:
-  PnaclResources(Plugin* plugin, PnaclCoordinator* coordinator)
+  PnaclResources(Plugin* plugin,
+                 PnaclCoordinator* coordinator,
+                 const nacl::string& resource_base_url)
       : plugin_(plugin),

[robertm, 2011/12/13 17:06:39]

pre-existing
the dependency on both the plugin and the coordiator create coupling which would
be nice to avoid.

maybe at least the plugin dependency can be avoided.
by offering additional proxy functions in the coordinator,
similar to ReportPpapiError()

This makes the coordinator a little more ugly but I think it would be worth it,
concretely:
the sequence  GetLoadedFileDesc() ->  plugin_->wrapper_factory()->MakeFileDesc()
should be wrapped in coordinator proxy function

and plugin_->StreamAsFile() into another one

[sehr (please use chromium), 2011/12/13 20:05:04]

See above.

         coordinator_(coordinator),
         all_loaded_(true),
-        client_callback_is_valid_(false)
-        { }
+        client_callback_is_valid_(false),
+        resource_base_url_(resource_base_url) {
+    callback_factory_.Initialize(this);
+  }
 
   virtual ~PnaclResources();
 
-  void Initialize();
-
   // URLs are all relative to the coordinator's resource_base_url().
 
   // Get the wrapper for the downloaded resource.

[robertm, 2011/12/13 17:06:39]

when is it ok to call this function?

[sehr (please use chromium), 2011/12/13 20:05:04]

Comment added.

   nacl::DescWrapper* WrapperForUrl(const nacl::string& url) {
     return resource_wrappers_[url];
   }
 
   // Add an URL for download.

[robertm, 2011/12/13 17:06:39]

mention: do not call this after StartDownloads() has been called

[sehr (please use chromium), 2011/12/13 20:05:04]

I removed this interface, adding a vector of URLs to the constructor, as you
suggested.

   void AddResourceUrl(const nacl::string& url);
   // Start fetching the URLs.
   void StartDownloads();
   // Set the callback for what to do when all the resources are available.
   void RunWhenAllLoaded(pp::CompletionCallback& client_callback);

[robertm, 2011/12/13 17:06:39]

This maybe irrelevant once the class protocol has been clarified:

if this is required please add to contructor or to  StartDownloads()

[sehr (please use chromium), 2011/12/13 20:05:04]

Done.

 
  private:
   NACL_DISALLOW_COPY_AND_ASSIGN(PnaclResources);
 
   // Callback invoked each time one resource has been loaded.
   void ResourceReady(int32_t pp_error,
                      const nacl::string& url,
                      const nacl::string& full_url);
   // Callback invoked when all resources have been loaded.
   void AllLoaded(int32_t pp_error);
 
   Plugin* plugin_;
   PnaclCoordinator* coordinator_;
   std::vector<nacl::string> resource_urls_;
   std::map<nacl::string, nacl::DescWrapper*> resource_wrappers_;
   nacl::scoped_ptr<DelayedCallback> delayed_callback_;
   // Set once all resources have been loaded to indicate that future calls to
   // RunWhenAllLoaded should immediately invoke the client callback.  This
   // simplifies the client while allowing resources to be used for subsequent
   // translations.
   bool all_loaded_;
   // Callback to be invoked when all resources can be guaranteed available.
   pp::CompletionCallback client_callback_;
   // If RunWhenAllLoaded was called before all resources have been loaded,
   // this indicates that the registered client callback should be used when
   // the last resource arrives.
   bool client_callback_is_valid_;
+  // The base url for looking up resources.
+  nacl::string resource_base_url_;
   // Factory for ready callbacks, etc.
   pp::CompletionCallbackFactory<PnaclResources> callback_factory_;
 };
 
 }  // namespace plugin;
 #endif  // NATIVE_CLIENT_SRC_TRUSTED_PLUGIN_PNACL_RESOURCES_H_