<webview>: Implement support for package-local chrome-extension:// URLs

chrome/browser/profiles/profile_impl_io_data.cc

Index: chrome/browser/profiles/profile_impl_io_data.cc
diff --git a/chrome/browser/profiles/profile_impl_io_data.cc b/chrome/browser/profiles/profile_impl_io_data.cc
index a096dd17c7b32a2df962dbdce61b56194fbd7778..dd912032b87b01c657440b7f91080391c9b8a2fe 100644
--- a/chrome/browser/profiles/profile_impl_io_data.cc
+++ b/chrome/browser/profiles/profile_impl_io_data.cc
@@ -18,6 +18,7 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/custom_handlers/protocol_handler_registry.h"
 #include "chrome/browser/custom_handlers/protocol_handler_registry_factory.h"
+#include "chrome/browser/extensions/extension_protocols.h"
 #include "chrome/browser/io_thread.h"
 #include "chrome/browser/net/chrome_net_log.h"
 #include "chrome/browser/net/chrome_network_delegate.h"
@@ -593,6 +594,10 @@ ProfileImplIOData::InitializeAppRequestContext(
         network_delegate(),
         ftp_factory_.get());
   } else {
+    job_factory->SetProtocolHandler(

[Charlie Reis, 2013/09/24 19:41:55]

This doesn't make sense to me.

First, the comment suggests that the first branch is for non-guests and the
second branch is for guests.  In that case, it might make sense to selectively
create the extension protocol handler (and not the rest of
SetUpJobFactoryDefaults) here, though we should document that's what we're
doing.

Indeed, that reflects how the code was originally written:
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/profiles/profi...

However, the check is now about whether the partition is in_memory or not, as of
Nasko's https://chromiumcodereview.appspot.com/11147026.  That means guests
could hit either branch (based on whether the guest is persistant), and
non-guests could hit either branch.  That means this check isn't correct for
what it's being used for.

Nasko, perhaps you could help us understand what the right thing to do is?  Was
this check just a temporary thing before guests could have persistent storage?

[nasko, 2013/09/24 20:02:01]

This indeed looks incorrect and I think it goes back to the original patch
Charlie is pointing to. The logic here (protocol handling) shouldn't be based on
whether a partition is in-memory or not. Protocol handling should be based on
whether this is for a guest process or not, so I think we should fix this here.

[Fady Samuel, 2013/09/24 23:05:02]

Looks like profile_impl_io_data doesn't have enough info to determine for
something is a guest? This looks like it needs a bunch of refactoring. It
probably doesn't make sense to do that refactoring in this CL. Shall I do it in
a separate CL and add a TODO here? Shall I write a separate CL and land that
first?

+          extensions::kExtensionScheme,
+          CreateExtensionProtocolHandler(is_incognito(),
+                                          GetExtensionInfoMap()));

[Charlie Reis, 2013/09/24 19:41:55]

nit: 598-600 all have too much indent.

[Fady Samuel, 2013/09/24 23:05:02]

Done.

     top_job_factory = job_factory.PassAs<net::URLRequestJobFactory>();
   }
   context->SetJobFactory(top_job_factory.Pass());