Committing patch 256001 for Roger:http://codereview.chromium.org/256001Allow Chrome Frame to display chrome-extension URLs when running in privilegedmode.BUG=noneTEST=see unit tests

Review URL: http://codereview.chromium.org/246050

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@27724 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 64 insertions, 25 deletions

diff --git a/chrome_frame/bho.cc b/chrome_frame/bho.cc
index 9561cc1..52d19ba 100644
--- a/chrome_frame/bho.cc
+++ b/chrome_frame/bho.cc
@@ -88,7 +88,7 @@ STDMETHODIMP Bho::BeforeNavigate2(IDispatch* dispatch, VARIANT* url,
 
     std::wstring current_url;
     bool is_chrome_protocol = false;
-    if (is_top_level && IsValidUrlScheme(url->bstrVal)) {
+    if (is_top_level && IsValidUrlScheme(url->bstrVal, false)) {
       current_url.assign(url->bstrVal, SysStringLen(url->bstrVal));
       is_chrome_protocol = StartsWith(current_url, kChromeProtocolPrefix,
                                       false);
diff --git a/chrome_frame/chrome_active_document.cc b/chrome_frame/chrome_active_document.cc
index 147ef0b..ad104c9 100644
--- a/chrome_frame/chrome_active_document.cc
+++ b/chrome_frame/chrome_active_document.cc
@@ -677,7 +677,7 @@ bool ChromeActiveDocument::ParseUrl(const std::wstring& url,
         !StartsWith(initial_url, kChromeAttachExternalTabPrefix, false);
   }
 
-  if (!IsValidUrlScheme(initial_url)) {
+  if (!IsValidUrlScheme(initial_url, is_privileged_)) {
     DLOG(WARNING) << __FUNCTION__ << " Disallowing navigation to url: "
                   << url;
     return false;
@@ -721,7 +721,7 @@ bool ChromeActiveDocument::LaunchUrl(const std::wstring& url,
       if (url_.Length()) {
         std::string utf8_url;
         WideToUTF8(url_, url_.Length(), &utf8_url);
-        if (!automation_client_->InitiateNavigation(utf8_url)) {
+        if (!automation_client_->InitiateNavigation(utf8_url, is_privileged_)) {
           DLOG(ERROR) << "Invalid URL: " << url;
           Error(L"Invalid URL");
           url_.Reset();
diff --git a/chrome_frame/chrome_frame_activex_base.h b/chrome_frame/chrome_frame_activex_base.h
index bfdabb0..7754bc0 100644
--- a/chrome_frame/chrome_frame_activex_base.h
+++ b/chrome_frame/chrome_frame_activex_base.h
@@ -10,7 +10,7 @@
 #include <atlctl.h>
 
 // Copied min/max defs from windows headers to appease atlimage.h.
-// TODO(slightlyoff): Figure out of more recent platform SDK's (> 6.1) 
+// TODO(slightlyoff): Figure out of more recent platform SDK's (> 6.1)
 //   undo the janky "#define NOMINMAX" train wreck. See:
 // http://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=100703
 #ifndef max
@@ -282,13 +282,13 @@ END_MSG_MAP()
     return false;
   }
 
-  // Should connections initiated by this class try to block 
-  // responses served with the X-Frame-Options header? 
-  // ActiveX controls genereally will want to do this, 
-  // returning true, while true top-level documents 
-  // (ActiveDocument servers) will not. Your specialization 
-  // of this template should implement this method based on how 
-  // it "feels" from a security perspective. If it's hosted in another 
+  // Should connections initiated by this class try to block
+  // responses served with the X-Frame-Options header?
+  // ActiveX controls genereally will want to do this,
+  // returning true, while true top-level documents
+  // (ActiveDocument servers) will not. Your specialization
+  // of this template should implement this method based on how
+  // it "feels" from a security perspective. If it's hosted in another
   // scriptable document, return true, else false.
   virtual bool is_frame_busting_enabled() const {
     return true;
@@ -509,7 +509,7 @@ END_MSG_MAP()
     // We can initiate navigation here even if ready_state is not complete.
     // We do not have to set proxy, and AutomationClient will take care
     // of navigation just after CreateExternalTab is done.
-    if (!automation_client_->InitiateNavigation(full_url)) {
+    if (!automation_client_->InitiateNavigation(full_url, is_privileged_)) {
       // TODO(robertshield): Make InitiateNavigation return more useful
       // error information.
       return E_INVALIDARG;
diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc
index 1f369b5..a03a39c 100644
--- a/chrome_frame/chrome_frame_automation.cc
+++ b/chrome_frame/chrome_frame_automation.cc
@@ -477,15 +477,17 @@ void ChromeFrameAutomationClient::Uninitialize() {
   init_state_ = UNINITIALIZED;
 }
 
-bool ChromeFrameAutomationClient::InitiateNavigation(const std::string& url) {
+bool ChromeFrameAutomationClient::InitiateNavigation(const std::string& url,
+                                                     bool is_privileged) {
   if (url.empty())
     return false;
 
   url_ = GURL(url);
 
   // Catch invalid URLs early.
-  if (!url_.is_valid() || !IsValidUrlScheme(UTF8ToWide(url))) {
-    DLOG(ERROR) << "Invalid URL passed to InitiateNavigation: " << url;
+  if (!url_.is_valid() || !IsValidUrlScheme(UTF8ToWide(url), is_privileged)) {
+    DLOG(ERROR) << "Invalid URL passed to InitiateNavigation: " << url
+                << " is_privileged=" << is_privileged;
     return false;
   }
 
diff --git a/chrome_frame/chrome_frame_automation.h b/chrome_frame/chrome_frame_automation.h
index 796facb..dfe408d 100644
--- a/chrome_frame/chrome_frame_automation.h
+++ b/chrome_frame/chrome_frame_automation.h
@@ -189,7 +189,7 @@ class ChromeFrameAutomationClient
                           bool incognito);
   void Uninitialize();
 
-  virtual bool InitiateNavigation(const std::string& url);
+  virtual bool InitiateNavigation(const std::string& url, bool is_privileged);
   virtual bool NavigateToIndex(int index);
   bool ForwardMessageFromExternalHost(const std::string& message,
                                       const std::string& origin,
diff --git a/chrome_frame/chrome_frame_npapi.cc b/chrome_frame/chrome_frame_npapi.cc
index 0c58cff..f558fdd 100644
--- a/chrome_frame/chrome_frame_npapi.cc
+++ b/chrome_frame/chrome_frame_npapi.cc
@@ -409,7 +409,7 @@ bool ChromeFrameNPAPI::Initialize(NPMIMEType mime_type, NPP instance,
   // part of LaunchSettings
   /*
   if (!src_.empty())
-    automation_client_->InitiateNavigation(src_);
+    automation_client_->InitiateNavigation(src_, is_privileged_);
 
   std::string proxy_settings;
   bool has_prefs = pref_service_->Initialize(instance_,
@@ -1041,7 +1041,7 @@ void ChromeFrameNPAPI::OnAutomationServerReady() {
   }
 
   if (!src_.empty()) {
-    if (!automation_client_->InitiateNavigation(src_)) {
+    if (!automation_client_->InitiateNavigation(src_, is_privileged_)) {
       DLOG(ERROR) << "Failed to navigate to: " << src_;
       src_.clear();
     }
@@ -1309,7 +1309,7 @@ bool ChromeFrameNPAPI::NavigateToURL(const NPVariant* args, uint32_t arg_count,
   src_ = full_url;
   // Navigate only if we completed initialization i.e. proxy is set etc.
   if (ready_state_ == READYSTATE_COMPLETE) {
-    if (!automation_client_->InitiateNavigation(full_url)) {
+    if (!automation_client_->InitiateNavigation(full_url, is_privileged_)) {
       // TODO(tommi): call NPN_SetException.
       src_.clear();
       return false;
diff --git a/chrome_frame/test/chrome_frame_automation_mock.h b/chrome_frame/test/chrome_frame_automation_mock.h
index 4c3fe7b..7678282 100644
--- a/chrome_frame/test/chrome_frame_automation_mock.h
+++ b/chrome_frame/test/chrome_frame_automation_mock.h
@@ -41,7 +41,7 @@ class AutomationMockDelegate
   // Navigate external tab to the specified url through automation
   bool Navigate(const std::string& url) {
     url_ = GURL(url);
-    return automation_client_->InitiateNavigation(url);
+    return automation_client_->InitiateNavigation(url, false);
   }
 
   // Navigate the external to a 'file://' url for unit test files
diff --git a/chrome_frame/test/chrome_frame_unittests.cc b/chrome_frame/test/chrome_frame_unittests.cc
index d24f5b3..206c6d6 100644
--- a/chrome_frame/test/chrome_frame_unittests.cc
+++ b/chrome_frame/test/chrome_frame_unittests.cc
@@ -1101,7 +1101,7 @@ TEST(CFACWithChrome, NavigateOk) {
   EXPECT_CALL(cfd, OnAutomationServerReady())
       .WillOnce(testing::InvokeWithoutArgs(TaskHolder(NewRunnableMethod(
           client.get(), &ChromeFrameAutomationClient::InitiateNavigation,
-          url))));
+          url, false))));
 
 //  cfd.SetOnNavigationStateChanged();
   EXPECT_CALL(cfd,
@@ -1141,7 +1141,7 @@ TEST(CFACWithChrome, DISABLED_NavigateFailed) {
   EXPECT_CALL(cfd, OnAutomationServerReady())
       .WillOnce(testing::InvokeWithoutArgs(TaskHolder(NewRunnableMethod(
           client.get(), &ChromeFrameAutomationClient::InitiateNavigation,
-          url))));
+          url, false))));
 
   EXPECT_CALL(cfd,
     OnNavigationStateChanged(testing::_, testing::_))
@@ -1199,7 +1199,7 @@ TEST(CFACWithChrome, UseHostNetworkStack) {
   EXPECT_CALL(cfd, OnAutomationServerReady())
       .WillOnce(testing::InvokeWithoutArgs(TaskHolder(NewRunnableMethod(
           client.get(), &ChromeFrameAutomationClient::InitiateNavigation,
-          url))));
+          url, false))));
 
   EXPECT_CALL(cfd, OnNavigationStateChanged(testing::_, testing::_))
       .Times(testing::AnyNumber());
diff --git a/chrome_frame/test/util_unittests.cc b/chrome_frame/test/util_unittests.cc
index cc2727b..3f07b82 100644
--- a/chrome_frame/test/util_unittests.cc
+++ b/chrome_frame/test/util_unittests.cc
@@ -118,3 +118,36 @@ TEST(UtilTests, HaveSameOrigin) {
     EXPECT_EQ(test.same_origin, HaveSameOrigin(test.a, test.b));
   }
 }
+
+TEST(UtilTests, IsValidUrlScheme) {
+  struct Cases {
+    const wchar_t* url;
+    bool is_privileged;
+    bool expected;
+  } test_cases[] = {
+    // non-privileged test cases
+    { L"http://www.google.ca", false, true },
+    { L"https://www.google.ca", false, true },
+    { L"about:config", false, true },
+    { L"view-source:http://www.google.ca", false, true },
+    { L"chrome-extension://aaaaaaaaaaaaaaaaaaa/toolstrip.html", false, false },
+    { L"ftp://www.google.ca", false, false },
+    { L"file://www.google.ca", false, false },
+    { L"file://C:\boot.ini", false, false },
+
+    // privileged test cases
+    { L"http://www.google.ca", true, true },
+    { L"https://www.google.ca", true, true },
+    { L"about:config", true, true },
+    { L"view-source:http://www.google.ca", true, true },
+    { L"chrome-extension://aaaaaaaaaaaaaaaaaaa/toolstrip.html", true, true },
+    { L"ftp://www.google.ca", true, false },
+    { L"file://www.google.ca", true, false },
+    { L"file://C:\boot.ini", true, false },
+  };
+
+  for (int i = 0; i < arraysize(test_cases); ++i) {
+    const Cases& test = test_cases[i];
+    EXPECT_EQ(test.expected, IsValidUrlScheme(test.url, test.is_privileged));
+  }
+}
diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc
index b445c26..36e8afd 100644
--- a/chrome_frame/utils.cc
+++ b/chrome_frame/utils.cc
@@ -601,7 +601,7 @@ HRESULT GetUrlFromMoniker(IMoniker* moniker, IBindCtx* bind_context,
   return hr;
 }
 
-bool IsValidUrlScheme(const std::wstring& url) {
+bool IsValidUrlScheme(const std::wstring& url, bool is_privileged) {
   if (url.empty())
     return false;
 
@@ -611,6 +611,9 @@ bool IsValidUrlScheme(const std::wstring& url) {
       crack_url.SchemeIs("about") || crack_url.SchemeIs("view-source"))
     return true;
 
+  if (is_privileged && crack_url.SchemeIs("chrome-extension"))
+    return true;
+
   if (StartsWith(url, kChromeAttachExternalTabPrefix, false))
     return true;
 
diff --git a/chrome_frame/utils.h b/chrome_frame/utils.h
index 80b9a53..baece31 100644
--- a/chrome_frame/utils.h
+++ b/chrome_frame/utils.h
@@ -219,7 +219,8 @@ HRESULT GetUrlFromMoniker(IMoniker* moniker, IBindCtx* bind_context,
 
 // Returns true if the URL passed in is something which can be handled by
 // Chrome. If this function returns false then we should fail the navigation.
-bool IsValidUrlScheme(const std::wstring& url);
+// When is_privileged is true, chrome extension URLs will be considered valid.
+bool IsValidUrlScheme(const std::wstring& url, bool is_privileged);
 
 // This returns the base directory in which to store user profiles.
 bool GetUserProfileBaseDirectory(std::wstring* path);