Browse Source

Revert "protocol: use streaming interface between fetcher and requestjob"

This reverts commit 71b5d946e0f59251e36049cd51040c8825506579.
Kevin Sawicki 8 years ago
parent
commit
c43c3b3d80

+ 63 - 79
atom/browser/net/url_request_fetch_job.cc

@@ -9,18 +9,14 @@
 
 #include "atom/browser/api/atom_api_session.h"
 #include "atom/browser/atom_browser_context.h"
-#include "base/guid.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
-#include "brightray/browser/url_request_context_getter.h"
-#include "content/browser/streams/stream_context.h"
 #include "native_mate/dictionary.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_response_writer.h"
-#include "url/url_constants.h"
 
 using content::BrowserThread;
 
@@ -66,8 +62,7 @@ class ResponsePiper : public net::URLFetcherResponseWriter {
       job_->HeadersCompleted();
       first_write_ = false;
     }
-    job_->stream()->AddData(buffer->data(), num_bytes);
-    return num_bytes;
+    return job_->DataAvailable(buffer, num_bytes, callback);
   }
   int Finish(int net_error, const net::CompletionCallback& callback) override {
     return net::OK;
@@ -82,11 +77,12 @@ class ResponsePiper : public net::URLFetcherResponseWriter {
 
 }  // namespace
 
-URLRequestFetchJob::URLRequestFetchJob(net::URLRequest* request,
-                                       net::NetworkDelegate* network_delegate)
+URLRequestFetchJob::URLRequestFetchJob(
+    net::URLRequest* request, net::NetworkDelegate* network_delegate)
     : JsAsker<net::URLRequestJob>(request, network_delegate),
       pending_buffer_size_(0),
-      total_bytes_read_(0) {}
+      write_num_bytes_(0) {
+}
 
 void URLRequestFetchJob::BeforeStartInUI(
     v8::Isolate* isolate, v8::Local<v8::Value> value) {
@@ -173,22 +169,7 @@ void URLRequestFetchJob::StartAsync(std::unique_ptr<base::Value> options) {
   fetcher_->SetExtraRequestHeaders(
       request()->extra_request_headers().ToString());
 
-  // Create readable stream for URLFetcher response.
-  content::StreamContext* stream_context =
-      static_cast<brightray::URLRequestContextGetter*>(request_context_getter())
-          ->stream_context();
-
-  if (stream_context) {
-    GURL stream_url(std::string(url::kBlobScheme) + ":" +
-                    formated_url.GetOrigin().spec() + base::GenerateGUID());
-    stream_ =
-        new content::Stream(stream_context->registry(), nullptr, stream_url);
-    stream_->SetReadObserver(this);
-    fetcher_->Start();
-  } else {
-    NotifyStartError(net::URLRequestStatus(net::URLRequestStatus::CANCELED,
-                                           net::ERR_ABORTED));
-  }
+  fetcher_->Start();
 }
 
 void URLRequestFetchJob::HeadersCompleted() {
@@ -197,55 +178,53 @@ void URLRequestFetchJob::HeadersCompleted() {
   NotifyHeadersComplete();
 }
 
+int URLRequestFetchJob::DataAvailable(net::IOBuffer* buffer,
+                                      int num_bytes,
+                                      const net::CompletionCallback& callback) {
+  // When pending_buffer_ is empty, there's no ReadRawData() operation waiting
+  // for IO completion, we have to save the parameters until the request is
+  // ready to read data.
+  if (!pending_buffer_.get()) {
+    write_buffer_ = buffer;
+    write_num_bytes_ = num_bytes;
+    write_callback_ = callback;
+    return net::ERR_IO_PENDING;
+  }
+
+  // Write data to the pending buffer and clear them after the writing.
+  int bytes_read = BufferCopy(buffer, num_bytes,
+                              pending_buffer_.get(), pending_buffer_size_);
+  ClearPendingBuffer();
+  ReadRawDataComplete(bytes_read);
+  return bytes_read;
+}
+
 void URLRequestFetchJob::Kill() {
   JsAsker<URLRequestJob>::Kill();
-  ClearStream();
   fetcher_.reset();
 }
 
-void URLRequestFetchJob::OnDataAvailable(content::Stream* stream) {
-  if (!pending_buffer_.get())
-    return;
-
-  int result = 0;
-  auto state = stream_->ReadRawData(pending_buffer_.get(), pending_buffer_size_,
-                                    &result);
-  if (state == content::Stream::STREAM_ABORTED)
-    result = net::ERR_CONNECTION_RESET;
-
-  // Clear the buffers before notifying the read is complete, so that it is
-  // safe for the observer to read.
-  pending_buffer_ = nullptr;
-  pending_buffer_size_ = 0;
-
-  if (result > 0)
-    total_bytes_read_ += result;
-  ReadRawDataComplete(result);
-}
-
 int URLRequestFetchJob::ReadRawData(net::IOBuffer* dest, int dest_size) {
   if (GetResponseCode() == 204) {
     request()->set_received_response_content_length(prefilter_bytes_read());
     return net::OK;
   }
 
-  int bytes_read = 0;
-  switch (stream_->ReadRawData(dest, dest_size, &bytes_read)) {
-    case content::Stream::STREAM_HAS_DATA:
-      total_bytes_read_ += bytes_read;
-      return bytes_read;
-    case content::Stream::STREAM_COMPLETE:
-      return stream_->GetStatus();
-    case content::Stream::STREAM_EMPTY:
-      pending_buffer_ = dest;
-      pending_buffer_size_ = dest_size;
-      return net::ERR_IO_PENDING;
-    case content::Stream::STREAM_ABORTED:
-      // Handle this as connection reset.
-      return net::ERR_CONNECTION_RESET;
+  // When write_buffer_ is empty, there is no data valable yet, we have to save
+  // the dest buffer util DataAvailable.
+  if (!write_buffer_.get()) {
+    pending_buffer_ = dest;
+    pending_buffer_size_ = dest_size;
+    return net::ERR_IO_PENDING;
   }
-  NOTREACHED();
-  return net::ERR_FAILED;
+
+  // Read from the write buffer and clear them after reading.
+  int bytes_read = BufferCopy(write_buffer_.get(), write_num_bytes_,
+                              dest, dest_size);
+  net::CompletionCallback write_callback = write_callback_;
+  ClearWriteBuffer();
+  write_callback.Run(bytes_read);
+  return bytes_read;
 }
 
 bool URLRequestFetchJob::GetMimeType(std::string* mime_type) const {
@@ -267,18 +246,11 @@ int URLRequestFetchJob::GetResponseCode() const {
   return response_info_->headers->response_code();
 }
 
-int64_t URLRequestFetchJob::GetTotalReceivedBytes() const {
-  int64_t total_received_bytes = 0;
-  if (response_info_)
-    total_received_bytes = response_info_->headers->raw_headers().size();
-  if (stream_.get())
-    total_received_bytes += total_bytes_read_;
-  return total_received_bytes;
-}
-
 void URLRequestFetchJob::OnURLFetchComplete(const net::URLFetcher* source) {
-  auto status = fetcher_->GetStatus();
-  if (status.is_success()) {
+  ClearPendingBuffer();
+  ClearWriteBuffer();
+
+  if (fetcher_->GetStatus().is_success()) {
     if (!response_info_) {
       // Since we notify header completion only after first write there will be
       // no response object constructed for http respones with no content 204.
@@ -286,16 +258,28 @@ void URLRequestFetchJob::OnURLFetchComplete(const net::URLFetcher* source) {
       HeadersCompleted();
       return;
     }
-    stream_->Finalize(0);
+    ReadRawDataComplete(0);
   } else {
-    stream_->Finalize(status.error());
-    NotifyStartError(status);
+    NotifyStartError(fetcher_->GetStatus());
   }
 }
 
-void URLRequestFetchJob::ClearStream() {
-  stream_->RemoveReadObserver(this);
-  stream_ = nullptr;
+int URLRequestFetchJob::BufferCopy(net::IOBuffer* source, int num_bytes,
+                                   net::IOBuffer* target, int target_size) {
+  int bytes_written = std::min(num_bytes, target_size);
+  memcpy(target->data(), source->data(), bytes_written);
+  return bytes_written;
+}
+
+void URLRequestFetchJob::ClearPendingBuffer() {
+  pending_buffer_ = nullptr;
+  pending_buffer_size_ = 0;
+}
+
+void URLRequestFetchJob::ClearWriteBuffer() {
+  write_buffer_ = nullptr;
+  write_num_bytes_ = 0;
+  write_callback_.Reset();
 }
 
 }  // namespace atom

+ 13 - 11
atom/browser/net/url_request_fetch_job.h

@@ -17,15 +17,15 @@ namespace atom {
 
 class URLRequestFetchJob : public JsAsker<net::URLRequestJob>,
                            public net::URLFetcherDelegate,
-                           public brightray::URLRequestContextGetter::Delegate,
-                           public content::StreamReadObserver {
+                           public brightray::URLRequestContextGetter::Delegate {
  public:
   URLRequestFetchJob(net::URLRequest*, net::NetworkDelegate*);
 
   // Called by response writer.
   void HeadersCompleted();
-
-  content::Stream* stream() const { return stream_.get(); }
+  int DataAvailable(net::IOBuffer* buffer,
+                    int num_bytes,
+                    const net::CompletionCallback& callback);
 
  protected:
   // JsAsker:
@@ -38,26 +38,28 @@ class URLRequestFetchJob : public JsAsker<net::URLRequestJob>,
   bool GetMimeType(std::string* mime_type) const override;
   void GetResponseInfo(net::HttpResponseInfo* info) override;
   int GetResponseCode() const override;
-  int64_t GetTotalReceivedBytes() const override;
 
   // net::URLFetcherDelegate:
   void OnURLFetchComplete(const net::URLFetcher* source) override;
 
-  // content::StreamReadObserver:
-  void OnDataAvailable(content::Stream* stream) override;
-
  private:
-  void ClearStream();
+  int BufferCopy(net::IOBuffer* source, int num_bytes,
+                 net::IOBuffer* target, int target_size);
+  void ClearPendingBuffer();
+  void ClearWriteBuffer();
 
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
   std::unique_ptr<net::URLFetcher> fetcher_;
   std::unique_ptr<net::HttpResponseInfo> response_info_;
-  scoped_refptr<content::Stream> stream_;
 
   // Saved arguments passed to ReadRawData.
   scoped_refptr<net::IOBuffer> pending_buffer_;
   int pending_buffer_size_;
-  int total_bytes_read_;
+
+  // Saved arguments passed to DataAvailable.
+  scoped_refptr<net::IOBuffer> write_buffer_;
+  int write_num_bytes_;
+  net::CompletionCallback write_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(URLRequestFetchJob);
 };

+ 0 - 5
brightray/browser/browser_context.cc

@@ -19,7 +19,6 @@
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/pref_service_factory.h"
-#include "content/browser/streams/stream_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/storage_partition.h"
@@ -162,10 +161,6 @@ MediaDeviceIDSalt* BrowserContext::GetMediaDeviceIDSalt() {
   return media_device_id_salt_.get();
 }
 
-content::StreamContext* BrowserContext::GetStreamContext() {
-  return content::StreamContext::GetFor(this);
-}
-
 base::FilePath BrowserContext::GetPath() const {
   return path_;
 }

+ 0 - 1
brightray/browser/browser_context.h

@@ -89,7 +89,6 @@ class BrowserContext : public base::RefCounted<BrowserContext>,
   // URLRequestContextGetter::Delegate:
   net::NetworkDelegate* CreateNetworkDelegate() override;
   MediaDeviceIDSalt* GetMediaDeviceIDSalt() override;
-  content::StreamContext* GetStreamContext() override;
 
   base::FilePath GetPath() const override;
 

+ 0 - 8
brightray/browser/url_request_context_getter.h

@@ -21,10 +21,6 @@ namespace base {
 class MessageLoop;
 }
 
-namespace content {
-class StreamContext;
-}
-
 namespace net {
 class HostMappingRules;
 class HostResolver;
@@ -65,7 +61,6 @@ class URLRequestContextGetter : public net::URLRequestContextGetter {
       return nullptr;
     }
     virtual MediaDeviceIDSalt* GetMediaDeviceIDSalt() { return nullptr; }
-    virtual content::StreamContext* GetStreamContext() { return nullptr; }
   };
 
   URLRequestContextGetter(
@@ -90,9 +85,6 @@ class URLRequestContextGetter : public net::URLRequestContextGetter {
   MediaDeviceIDSalt* GetMediaDeviceIDSalt() const {
     return delegate_->GetMediaDeviceIDSalt();
   }
-  content::StreamContext* stream_context() const {
-    return delegate_->GetStreamContext();
-  }
 
  private:
   Delegate* delegate_;