Create complete URLRequestContextGetter for URLRequestFetchJob

The trivial one is causing crashes.

diff --git a/atom/browser/net/js_asker.cc b/atom/browser/net/js_asker.cc
index 8f0d1d2..b11a69c 100644 (file)
--- a/atom/browser/net/js_asker.cc
+++ b/atom/browser/net/js_asker.cc
@@ -16,7 +16,9 @@ namespace internal {
 namespace {
 
 // The callback which is passed to |handler|.
-void HandlerCallback(const ResponseCallback& callback, mate::Arguments* args) {
+void HandlerCallback(const BeforeStartCallback& before_start,
+                     const ResponseCallback& callback,
+                     mate::Arguments* args) {
   // If there is no argument passed then we failed.
   v8::Local<v8::Value> value;
   if (!args->GetNext(&value)) {
@@ -26,6 +28,9 @@ void HandlerCallback(const ResponseCallback& callback, mate::Arguments* args) {
     return;
   }
 
+  // Give the job a chance to parse V8 value.
+  before_start.Run(args->isolate(), value);
+
   // Pass whatever user passed to the actaul request job.
   V8ValueConverter converter;
   v8::Local<v8::Context> context = args->isolate()->GetCurrentContext();
@@ -40,15 +45,17 @@ void HandlerCallback(const ResponseCallback& callback, mate::Arguments* args) {
 void AskForOptions(v8::Isolate* isolate,
                    const JavaScriptHandler& handler,
                    net::URLRequest* request,
+                   const BeforeStartCallback& before_start,
                    const ResponseCallback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   v8::Locker locker(isolate);
   v8::HandleScope handle_scope(isolate);
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
   v8::Context::Scope context_scope(context);
-  handler.Run(request,
-              mate::ConvertToV8(isolate,
-                                base::Bind(&HandlerCallback, callback)));
+  handler.Run(
+      request,
+      mate::ConvertToV8(isolate,
+                        base::Bind(&HandlerCallback, before_start, callback)));
 }
 
 bool IsErrorOptions(base::Value* value, int* error) {

diff --git a/atom/browser/net/js_asker.h b/atom/browser/net/js_asker.h
index 061a9cb..8a70794 100644 (file)
--- a/atom/browser/net/js_asker.h
+++ b/atom/browser/net/js_asker.h
@@ -23,6 +23,8 @@ using JavaScriptHandler =
 
 namespace internal {
 
+using BeforeStartCallback =
+    base::Callback<void(v8::Isolate*, v8::Local<v8::Value>)>;
 using ResponseCallback =
     base::Callback<void(bool, scoped_ptr<base::Value> options)>;
 
@@ -30,6 +32,7 @@ using ResponseCallback =
 void AskForOptions(v8::Isolate* isolate,
                    const JavaScriptHandler& handler,
                    net::URLRequest* request,
+                   const BeforeStartCallback& before_start,
                    const ResponseCallback& callback);
 
 // Test whether the |options| means an error.
@@ -54,6 +57,7 @@ class JsAsker : public RequestJob {
   }
 
   // Subclass should do initailze work here.
+  virtual void BeforeStartInUI(v8::Isolate*, v8::Local<v8::Value>) {}
   virtual void StartAsync(scoped_ptr<base::Value> options) = 0;
 
   net::URLRequestContextGetter* request_context_getter() const {
@@ -69,6 +73,8 @@ class JsAsker : public RequestJob {
                    isolate_,
                    handler_,
                    RequestJob::request(),
+                   base::Bind(&JsAsker::BeforeStartInUI,
+                              weak_factory_.GetWeakPtr()),
                    base::Bind(&JsAsker::OnResponse,
                               weak_factory_.GetWeakPtr())));
   }

diff --git a/atom/browser/net/url_request_fetch_job.cc b/atom/browser/net/url_request_fetch_job.cc
index b176aef..d58ea3f 100644 (file)
--- a/atom/browser/net/url_request_fetch_job.cc
+++ b/atom/browser/net/url_request_fetch_job.cc
@@ -8,15 +8,14 @@
 #include <string>
 
 #include "base/strings/string_util.h"
-#include "base/thread_task_runner_handle.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 "net/url_request/url_request_context.h"
-#include "net/url_request/url_request_context_builder.h"
-#include "net/url_request/url_request_status.h"
+
+using content::BrowserThread;
 
 namespace atom {
 
@@ -81,6 +80,25 @@ URLRequestFetchJob::URLRequestFetchJob(
       pending_buffer_size_(0) {
 }
 
+void URLRequestFetchJob::BeforeStartInUI(
+    v8::Isolate* isolate, v8::Local<v8::Value> value) {
+  mate::Dictionary options;
+  if (!mate::ConvertFromV8(isolate, value, &options))
+    return;
+
+  // When |session| is set to |null| we use a new request context for fetch job.
+  // TODO(zcbenz): Handle the case when it is not null.
+  v8::Local<v8::Value> session;
+  if (options.Get("session", &session) && session->IsNull()) {
+    // We have to create the URLRequestContextGetter on UI thread.
+    url_request_context_getter_ = new brightray::URLRequestContextGetter(
+        this, nullptr, nullptr, base::FilePath(), true,
+        BrowserThread::UnsafeGetMessageLoopForThread(BrowserThread::IO),
+        BrowserThread::UnsafeGetMessageLoopForThread(BrowserThread::FILE),
+        nullptr, content::URLRequestInterceptorScopedVector());
+  }
+}
+
 void URLRequestFetchJob::StartAsync(scoped_ptr<base::Value> options) {
   if (!options->IsType(base::Value::TYPE_DICTIONARY)) {
     NotifyStartError(net::URLRequestStatus(
@@ -89,14 +107,12 @@ void URLRequestFetchJob::StartAsync(scoped_ptr<base::Value> options) {
   }
 
   std::string url, method, referrer;
-  base::Value* session = nullptr;
   base::DictionaryValue* upload_data = nullptr;
   base::DictionaryValue* dict =
       static_cast<base::DictionaryValue*>(options.get());
   dict->GetString("url", &url);
   dict->GetString("method", &method);
   dict->GetString("referrer", &referrer);
-  dict->Get("session", &session);
   dict->GetDictionary("uploadData", &upload_data);
 
   // Check if URL is valid.
@@ -117,9 +133,9 @@ void URLRequestFetchJob::StartAsync(scoped_ptr<base::Value> options) {
   fetcher_ = net::URLFetcher::Create(formated_url, request_type, this);
   fetcher_->SaveResponseWithWriter(make_scoped_ptr(new ResponsePiper(this)));
 
-  // When |session| is set to |null| we use a new request context for fetch job.
-  if (session && session->IsType(base::Value::TYPE_NULL))
-    fetcher_->SetRequestContext(CreateRequestContext());
+  // A request context getter is passed by the user.
+  if (url_request_context_getter_)
+    fetcher_->SetRequestContext(url_request_context_getter_.get());
   else
     fetcher_->SetRequestContext(request_context_getter());
 
@@ -144,18 +160,6 @@ void URLRequestFetchJob::StartAsync(scoped_ptr<base::Value> options) {
   fetcher_->Start();
 }
 
-net::URLRequestContextGetter* URLRequestFetchJob::CreateRequestContext() {
-  if (!url_request_context_getter_.get()) {
-    auto task_runner = base::ThreadTaskRunnerHandle::Get();
-    net::URLRequestContextBuilder builder;
-    builder.set_proxy_service(net::ProxyService::CreateDirect());
-    request_context_ = builder.Build();
-    url_request_context_getter_ = new net::TrivialURLRequestContextGetter(
-        request_context_.get(), task_runner);
-  }
-  return url_request_context_getter_.get();
-}
-
 void URLRequestFetchJob::HeadersCompleted() {
   response_info_.reset(new net::HttpResponseInfo);
   response_info_->headers = fetcher_->GetResponseHeaders();

diff --git a/atom/browser/net/url_request_fetch_job.h b/atom/browser/net/url_request_fetch_job.h
index d9a247c..69067fd 100644 (file)
--- a/atom/browser/net/url_request_fetch_job.h
+++ b/atom/browser/net/url_request_fetch_job.h
@@ -8,6 +8,7 @@
 #include <string>
 
 #include "atom/browser/net/js_asker.h"
+#include "browser/url_request_context_getter.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "net/url_request/url_request_job.h"
@@ -17,7 +18,8 @@ namespace atom {
 class AtomBrowserContext;
 
 class URLRequestFetchJob : public JsAsker<net::URLRequestJob>,
-                           public net::URLFetcherDelegate {
+                           public net::URLFetcherDelegate,
+                           public brightray::URLRequestContextGetter::Delegate {
  public:
   URLRequestFetchJob(net::URLRequest*, net::NetworkDelegate*);
 
@@ -27,6 +29,7 @@ class URLRequestFetchJob : public JsAsker<net::URLRequestJob>,
 
  protected:
   // JsAsker:
+  void BeforeStartInUI(v8::Isolate*, v8::Local<v8::Value>) override;
   void StartAsync(scoped_ptr<base::Value> options) override;
 
   // net::URLRequestJob:
@@ -40,10 +43,6 @@ class URLRequestFetchJob : public JsAsker<net::URLRequestJob>,
   void OnURLFetchComplete(const net::URLFetcher* source) override;
 
  private:
-  // Create a independent request context.
-  net::URLRequestContextGetter* CreateRequestContext();
-
-  scoped_ptr<net::URLRequestContext> request_context_;
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
   scoped_ptr<net::URLFetcher> fetcher_;
   scoped_refptr<net::IOBuffer> pending_buffer_;

diff --git a/vendor/brightray b/vendor/brightray
index 7f9e25b..73d5b67 160000 (submodule)
--- a/vendor/brightray
+++ b/vendor/brightray
@@ -1 +1 @@
-Subproject commit 7f9e25b50b373aea5e7d0a50c33aea22c85ee876
+Subproject commit 73d5b67617e22c7d5a305d86a05ea40972683b63