AU: Manual proxy support

multi_http_fetcher.h

 // Copyright (c) 2010 The Chromium OS 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 CHROMEOS_PLATFORM_UPDATE_ENGINE_MULTI_HTTP_FETCHER_H__
 #define CHROMEOS_PLATFORM_UPDATE_ENGINE_MULTI_HTTP_FETCHER_H__
 
 #include <tr1/memory>
 #include <utility>
 #include <vector>
 
 #include "update_engine/http_fetcher.h"
 #include "update_engine/utils.h"
 
 // This class is a simple wrapper around an HttpFetcher. The client
 // specifies a vector of byte ranges. MultiHttpFetcher will fetch bytes
 // from those offsets. Pass -1 as a length to specify unlimited length.
 // It really only would make sense for the last range specified to have
 // unlimited length.
 
 namespace chromeos_update_engine {
 
 template<typename BaseHttpFetcher>

[petkov, 2010/11/19 05:37:15]

Longer term: this class might simpler if it didn't use templates and allocated
fetchers itself but rather implemented a method:

// takes ownership of |fetcher|.
AddFetcher(HttpFetcher* fetcher, off_t start, off_t size);

[petkov, 2010/11/19 17:10:04]

In fact, we can probably simplify this to reuse the same fetcher for each range.

[adlr, 2010/11/20 02:52:29]

Yeah, we could probably reuse fetchers, but we'd need to test that. Right now,
afaik, we've never reused a fetcher object.

 class MultiHttpFetcher : public HttpFetcher, public HttpFetcherDelegate {
  public:
   typedef std::vector<std::pair<off_t, off_t> > RangesVect;
+  typedef std::vector<std::tr1::shared_ptr<BaseHttpFetcher> > FetchersVect;
 
-  MultiHttpFetcher()
-      : sent_transfer_complete_(false),
+  MultiHttpFetcher(ProxyResolver* proxy_resolver)
+      : HttpFetcher(proxy_resolver),
+        sent_transfer_complete_(false),
         pending_next_fetcher_(false),
         current_index_(0),
         bytes_received_this_fetcher_(0) {}
   ~MultiHttpFetcher() {}
 
   void set_ranges(const RangesVect& ranges) {
     ranges_ = ranges;
     fetchers_.resize(ranges_.size());  // Allocate the fetchers
     for (typename std::vector<std::tr1::shared_ptr<BaseHttpFetcher>
              >::iterator it = fetchers_.begin(), e = fetchers_.end();
          it != e; ++it) {
-      (*it) = std::tr1::shared_ptr<BaseHttpFetcher>(new BaseHttpFetcher);
+      (*it) = std::tr1::shared_ptr<BaseHttpFetcher>(
+          new BaseHttpFetcher(proxy_resolver_));
       (*it)->set_delegate(this);
+      LOG(INFO) << "list of fetchers: " << (uint64_t)(it->get());

[petkov, 2010/11/19 05:37:15]

do you need this new logging? and below?

[adlr, 2010/11/20 02:52:29]

oops. yeah, when i use c-style casts i'm being quick for debugging... removed

     }
+    LOG(INFO) << "done w/ list";
   }
 
   void SetOffset(off_t offset) {}  // for now, doesn't support this
 
   // Begins the transfer to the specified URL.
   void BeginTransfer(const std::string& url) {
     url_ = url;
     if (ranges_.empty()) {
       if (delegate_)
         delegate_->TransferComplete(this, true);
@@ skipping 73 matching lines @@
       (*it)->SetConnectionAsExpensive(is_expensive);
     }
   }
   void SetBuildType(bool is_official) {
     for (typename std::vector<std::tr1::shared_ptr<BaseHttpFetcher> >::iterator
              it = fetchers_.begin(),
              e = fetchers_.end(); it != e; ++it) {
       (*it)->SetBuildType(is_official);
     }
   }
+  
+  virtual void SetProxies(const std::vector<std::string>& proxies) {

[petkov, 2010/11/19 05:37:15]

Instead of overloading this method, can't you just do SetProxies right before
calling BeginTransfer on the next fetcher? You might want to change SetProxies
to take begin/end iterators or implement set_proxies or switch deque to
vector...

[adlr, 2010/11/20 02:52:29]

We could do that. We would probably also want to do it w/ the other Set*()
methods, tho. Perhaps we should have an HttpFetcher cleanup to revamp all this?
For now I'm inclined to keep this inline w/ the other Set*() methods.
I tried to make SetProxies take iterators, but their type would be a template
type to the function, which makes it not able to be virtual. The nice thing
about having a deque is it handles large proxy lists better (arguably not a
likely case in practice). Just kind feels a little weird to "downgrade" to
vector...

[petkov, 2010/11/22 17:49:00]

Yeah, we should have a cleanup. And we could rename it to HTTPFetcher :-)
That's OK. The difference vs. the other Set* methods is that this one is
implemented in the base class (and also provides a data field in it).
You could also "upgrade" vector to deque :-) Doesn't matter much so feel free to
ignore.

+    for (typename FetchersVect::iterator it = fetchers_.begin(),
+             e = fetchers_.end();
+         it != e; ++it) {

[petkov, 2010/11/19 05:37:15]

move up?

[adlr, 2010/11/20 02:52:29]

Done.

+      (*it)->SetProxies(proxies);
+    }
+  }
 
  private:
   void SendTransferComplete(HttpFetcher* fetcher, bool successful) {
     if (sent_transfer_complete_)
       return;
     LOG(INFO) << "Sending transfer complete";
     sent_transfer_complete_ = true;
     http_response_code_ = fetcher->http_response_code();
     if (delegate_)
       delegate_->TransferComplete(this, successful);
   }
 
   void StartTransfer() {
     if (current_index_ >= ranges_.size()) {
       return;
     }
     LOG(INFO) << "Starting a transfer @" << ranges_[current_index_].first << "("
               << ranges_[current_index_].second << ")";
     bytes_received_this_fetcher_ = 0;
     fetchers_[current_index_]->SetOffset(ranges_[current_index_].first);
     if (delegate_)
       delegate_->SeekToOffset(ranges_[current_index_].first);
     fetchers_[current_index_]->BeginTransfer(url_);
   }
 
   void ReceivedBytes(HttpFetcher* fetcher, const char* bytes, int length) {
     TEST_AND_RETURN(current_index_ < ranges_.size());
     TEST_AND_RETURN(fetcher == fetchers_[current_index_].get());
     TEST_AND_RETURN(!pending_next_fetcher_);
+    if (fetcher != fetchers_[current_index_].get()) {

[petkov, 2010/11/19 05:37:15]

When is this true given that there's a TEST_AND_RETURN for this above?

[adlr, 2010/11/20 02:52:29]

i think this may have been an issue w/ a merge. removed.

+      LOG(INFO) << "current index: " << current_index_;
+      LOG(WARNING) << "Received bytes from invalid fetcher";
+      LOG(WARNING) << (uint64_t)(fetcher) << " exp " << (uint64_t)(fetchers_[current_index_].get());
+      return;
+    }
     off_t next_size = length;
     if (ranges_[current_index_].second >= 0) {
       next_size = std::min(next_size,
                            ranges_[current_index_].second -
                            bytes_received_this_fetcher_);
     }
     LOG_IF(WARNING, next_size <= 0) << "Asked to write length <= 0";
     if (delegate_) {
       delegate_->ReceivedBytes(this, bytes, next_size);
     }
@@ skipping 37 matching lines @@
   }
 
   // If true, do not send any more data or TransferComplete to the delegate.
   bool sent_transfer_complete_;
 
   // If true, the next fetcher needs to be started when TransferTerminated is
   // received from the current fetcher.
   bool pending_next_fetcher_;
 
   RangesVect ranges_;
-  std::vector<std::tr1::shared_ptr<BaseHttpFetcher> > fetchers_;
+  FetchersVect fetchers_;
 
   RangesVect::size_type current_index_;  // index into ranges_, fetchers_
   off_t bytes_received_this_fetcher_;
 
   DISALLOW_COPY_AND_ASSIGN(MultiHttpFetcher);
 };
 
 }  // namespace chromeos_update_engine
 
 #endif  // CHROMEOS_PLATFORM_UPDATE_ENGINE_MULTI_HTTP_FETCHER_H__