Add StoragePartition's ProtocolHandlers at URLRequestContext construction time.

chrome/browser/profiles/off_the_record_profile_io_data.cc

Index: chrome/browser/profiles/off_the_record_profile_io_data.cc
diff --git a/chrome/browser/profiles/off_the_record_profile_io_data.cc b/chrome/browser/profiles/off_the_record_profile_io_data.cc
index dfc9b81d85c57fe6f5db9353c81e74fba86d8061..7632f0dc0af209e7f87530db7bb2e0aa9a6e2709 100644
--- a/chrome/browser/profiles/off_the_record_profile_io_data.cc
+++ b/chrome/browser/profiles/off_the_record_profile_io_data.cc
@@ -84,10 +84,30 @@ OffTheRecordProfileIOData::Handle::GetMainRequestContextGetter() const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 #endif  // defined(OS_CHROMEOS)
   LazyInitialize();

[mmenke, 2012/12/20 16:55:06]

This call no longer makes any sense.

[pauljensen, 2012/12/20 20:24:07]

I think it still does.  If this function is the first Handle function to be
called it's necessary.  See my description of the intended initialization below.

-  if (!main_request_context_getter_) {
-    main_request_context_getter_ =
-        ChromeURLRequestContextGetter::CreateOffTheRecord(profile_, io_data_);
-  }
+  DCHECK(main_request_context_getter_);
+  return main_request_context_getter_;
+}
+
+scoped_refptr<ChromeURLRequestContextGetter>
+OffTheRecordProfileIOData::Handle::CreateMainRequestContextGetter(
+        scoped_ptr<net::URLRequestJobFactory::ProtocolHandler>
+            blob_protocol_handler,
+        scoped_ptr<net::URLRequestJobFactory::ProtocolHandler>
+            file_system_protocol_handler,
+        scoped_ptr<net::URLRequestJobFactory::Interceptor>
+            developer_protocol_handler) const {
+  // TODO(oshima): Re-enable when ChromeOS only accesses the profile on the UI
+  // thread.
+#if !defined(OS_CHROMEOS)
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+#endif  // defined(OS_CHROMEOS)
+  LazyInitialize();
+  DCHECK(!main_request_context_getter_);
+  main_request_context_getter_ =
+      ChromeURLRequestContextGetter::CreateOffTheRecord(
+          profile_, io_data_, blob_protocol_handler.Pass(),
+          file_system_protocol_handler.Pass(),
+          developer_protocol_handler.Pass());
   return main_request_context_getter_;
 }
 
@@ -115,8 +135,27 @@ OffTheRecordProfileIOData::Handle::GetIsolatedAppRequestContextGetter(
   StoragePartitionDescriptor descriptor(partition_path, in_memory);
   ChromeURLRequestContextGetterMap::iterator iter =
       app_request_context_getter_map_.find(descriptor);
-  if (iter != app_request_context_getter_map_.end())
-    return iter->second;
+  CHECK(iter != app_request_context_getter_map_.end());
+  return iter->second;
+}
+
+scoped_refptr<ChromeURLRequestContextGetter>
+OffTheRecordProfileIOData::Handle::CreateIsolatedAppRequestContextGetter(
+    const FilePath& partition_path,
+    bool in_memory,
+    scoped_ptr<net::URLRequestJobFactory::ProtocolHandler>
+        blob_protocol_handler,
+    scoped_ptr<net::URLRequestJobFactory::ProtocolHandler>
+        file_system_protocol_handler,
+    scoped_ptr<net::URLRequestJobFactory::Interceptor>
+        developer_protocol_handler) const {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!partition_path.empty());
+  LazyInitialize();

[mmenke, 2012/12/20 16:55:06]

This LazyInitialize doesn't work - looks like the
ChromeURLRequestContextGetter::CreateOffTheRecordForIsolatedApp gets the main
request context from the profile, so main_request_context_getter_ must already
be initialized, which implies LazyInitialize must already have been called.

[pauljensen, 2012/12/20 20:24:07]

It does work.  Here's the basic idea:
1. Someone calls one of these Handle::Create/Get functions which calls
LazyInitialize()
2. which calls io_data_->InitializeOnUIThread()
3. which calls BrowserContext::EnsureResourceContextInitialized()
4. which creates the main storage partition
5. which calls GetRequestContext() (in ToT) or CreateRequestContext() (in my CL)
6. which calls LazyInitialize() again, but it just returns because initialized_
is true
5. and then the main request context is created

Five functions end up reentrant.
Here's the relevant ToT call stack:
	ProfileImplIOData::Handle::GetMainRequestContextGetter() Line 151	C++
 	ProfileImpl::GetRequestContext() Line 801	C++
 	content::StoragePartitionImplMap::Get() Line 487	C++
 	content::`anonymous namespace'::GetStoragePartitionFromConfig() Line 64	C++
 	content::BrowserContext::GetStoragePartition() Line 149	C++
 	content::BrowserContext::GetDefaultStoragePartition() Line 181	C++
 	content::BrowserContext::EnsureResourceContextInitialized() Line 193	C++
 	ProfileIOData::InitializeOnUIThread() Line 242	C++
 	ProfileImplIOData::Handle::LazyInitialize() Line 285	C++
 	ProfileImplIOData::Handle::GetResourceContext() Line 135	C++
 	ProfileImpl::GetResourceContext() Line 837	C++
 	content::InitializeResourceContext() Line 64	C++
 	content::StoragePartitionImplMap::Get() Line 463	C++
 	content::`anonymous namespace'::GetStoragePartitionFromConfig() Line 64	C++
 	content::BrowserContext::GetStoragePartition() Line 149	C++
 	content::BrowserContext::GetDefaultStoragePartition() Line 181	C++
 	ProfileImpl::DoFinalInit() Line 527	C++
 	ProfileImpl::OnPrefsLoaded() Line 732	C++
 	ProfileImpl::ProfileImpl() Line 399	C++

[mmenke, 2012/12/20 21:12:54]

Looks like you're right, but I think this is about 5 times more complicated than
it needs to be.  I think something like:

static BrowserContext::AddProtocolHandlers(context, URLRequestFactory) (Or
BrowserContext::SetUpRequestFactory)

Would make a lot more sense.  Right now, this code repeatedly jumps between the
content/chrome boundary, which seems like a really bad idea, and has a rather
non-obvious call path.  The downside is that the new method would have to be
called explicitly, rather than being automatically called by creation of the
storage partition, but we already basically have that behavior, which is why
EnsureResourceContextInitialized has to be called in the first place.

Thoughts?

+
+  // Keep a map of request context getters, one per requested app ID.
+  StoragePartitionDescriptor descriptor(partition_path, in_memory);
+  DCHECK_EQ(app_request_context_getter_map_.count(descriptor), 0u);
 
   scoped_ptr<net::URLRequestJobFactory::Interceptor>
       protocol_handler_interceptor(
@@ -124,8 +163,9 @@ OffTheRecordProfileIOData::Handle::GetIsolatedAppRequestContextGetter(
               CreateURLInterceptor());
   ChromeURLRequestContextGetter* context =
       ChromeURLRequestContextGetter::CreateOffTheRecordForIsolatedApp(
-          profile_, io_data_, descriptor,
-          protocol_handler_interceptor.Pass());
+          profile_, io_data_, descriptor, protocol_handler_interceptor.Pass(),
+          blob_protocol_handler.Pass(), file_system_protocol_handler.Pass(),
+          developer_protocol_handler.Pass());
   app_request_context_getter_map_[descriptor] = context;
 
   return context;
@@ -153,8 +193,14 @@ OffTheRecordProfileIOData::~OffTheRecordProfileIOData() {
   DestroyResourceContext();
 }
 
-void OffTheRecordProfileIOData::LazyInitializeInternal(
-    ProfileParams* profile_params) const {
+void OffTheRecordProfileIOData::InitializeInternal(
+    ProfileParams* profile_params,
+    scoped_ptr<net::URLRequestJobFactory::ProtocolHandler>
+        blob_protocol_handler,
+    scoped_ptr<net::URLRequestJobFactory::ProtocolHandler>
+        file_system_protocol_handler,
+    scoped_ptr<net::URLRequestJobFactory::Interceptor>
+        developer_protocol_handler) const {
   ChromeURLRequestContext* main_context = main_request_context();
 
   IOThread* const io_thread = profile_params->io_thread;
@@ -217,6 +263,11 @@ void OffTheRecordProfileIOData::LazyInitializeInternal(
   scoped_ptr<net::URLRequestJobFactoryImpl> main_job_factory(
       new net::URLRequestJobFactoryImpl());
 
+  main_job_factory->SetProtocolHandler(
+      chrome::kBlobScheme, blob_protocol_handler.release());
+  main_job_factory->SetProtocolHandler(
+      chrome::kFileSystemScheme, file_system_protocol_handler.release());
+  main_job_factory->AddInterceptor(developer_protocol_handler.release());
   SetUpJobFactoryDefaults(
       main_job_factory.get(),
       profile_params->protocol_handler_interceptor.Pass(),
@@ -285,7 +336,13 @@ OffTheRecordProfileIOData::InitializeAppRequestContext(
     ChromeURLRequestContext* main_context,
     const StoragePartitionDescriptor& partition_descriptor,
     scoped_ptr<net::URLRequestJobFactory::Interceptor>
-        protocol_handler_interceptor) const {
+        protocol_handler_interceptor,
+    scoped_ptr<net::URLRequestJobFactory::ProtocolHandler>
+        blob_protocol_handler,
+    scoped_ptr<net::URLRequestJobFactory::ProtocolHandler>
+        file_system_protocol_handler,
+    scoped_ptr<net::URLRequestJobFactory::Interceptor>
+        developer_protocol_handler) const {
   AppRequestContext* context = new AppRequestContext(load_time_stats());
 
   // Copy most state from the main context.
@@ -308,6 +365,11 @@ OffTheRecordProfileIOData::InitializeAppRequestContext(
 
   scoped_ptr<net::URLRequestJobFactoryImpl> job_factory(
       new net::URLRequestJobFactoryImpl());
+  job_factory->SetProtocolHandler(chrome::kBlobScheme,
+                                  blob_protocol_handler.release());
+  job_factory->SetProtocolHandler(chrome::kFileSystemScheme,
+                                  file_system_protocol_handler.release());
+  job_factory->AddInterceptor(developer_protocol_handler.release());
   SetUpJobFactoryDefaults(job_factory.get(),
                           protocol_handler_interceptor.Pass(),
                           network_delegate(),
@@ -336,11 +398,20 @@ OffTheRecordProfileIOData::AcquireIsolatedAppRequestContext(
     ChromeURLRequestContext* main_context,
     const StoragePartitionDescriptor& partition_descriptor,
     scoped_ptr<net::URLRequestJobFactory::Interceptor>
-        protocol_handler_interceptor) const {
+        protocol_handler_interceptor,
+    scoped_ptr<net::URLRequestJobFactory::ProtocolHandler>
+        blob_protocol_handler,
+    scoped_ptr<net::URLRequestJobFactory::ProtocolHandler>
+        file_system_protocol_handler,
+    scoped_ptr<net::URLRequestJobFactory::Interceptor>
+        developer_protocol_handler) const {
   // We create per-app contexts on demand, unlike the others above.
   ChromeURLRequestContext* app_request_context =
       InitializeAppRequestContext(main_context, partition_descriptor,
-                                  protocol_handler_interceptor.Pass());
+                                  protocol_handler_interceptor.Pass(),
+                                  blob_protocol_handler.Pass(),
+                                  file_system_protocol_handler.Pass(),
+                                  developer_protocol_handler.Pass());
   DCHECK(app_request_context);
   return app_request_context;
 }