Allow browser to handle all WebUI navigations.

BUG=113496
TEST="Google Dashboard" link in Sync settings loads in new process.


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

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

7 files changed, 161 insertions, 21 deletions

diff --git a/content/renderer/media/audio_renderer_impl_unittest.cc b/content/renderer/media/audio_renderer_impl_unittest.cc
index 3123090..5320b1a 100644
--- a/content/renderer/media/audio_renderer_impl_unittest.cc
+++ b/content/renderer/media/audio_renderer_impl_unittest.cc
@@ -37,6 +37,8 @@ class MockRenderProcess : public RenderProcess {
       const gfx::Rect& rect) { return NULL; }
   virtual void ReleaseTransportDIB(TransportDIB* memory) {}
   virtual bool UseInProcessPlugins() const { return false; }
+  virtual void AddBindings(int bindings) {}
+  virtual int GetEnabledBindings() const { return 0; }
   virtual bool HasInitializedMediaLibrary() const { return false; }
 
  private:
diff --git a/content/renderer/render_process.h b/content/renderer/render_process.h
index 8ae6611..681b898 100644
--- a/content/renderer/render_process.h
+++ b/content/renderer/render_process.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -48,6 +48,13 @@ class RenderProcess : public ChildProcess {
   // Returns true if plugisn should be loaded in-process.
   virtual bool UseInProcessPlugins() const = 0;
 
+  // Keep track of the cumulative set of enabled bindings for this process,
+  // across any view.
+  virtual void AddBindings(int bindings) = 0;
+
+  // The cumulative set of enabled bindings for this process.
+  virtual int GetEnabledBindings() const = 0;
+
   // Returns a pointer to the RenderProcess singleton instance. Assuming that
   // we're actually a renderer or a renderer test, this static cast will
   // be correct.
diff --git a/content/renderer/render_process_impl.cc b/content/renderer/render_process_impl.cc
index fbb6417..ca56ce6 100644
--- a/content/renderer/render_process_impl.cc
+++ b/content/renderer/render_process_impl.cc
@@ -38,7 +38,8 @@ RenderProcessImpl::RenderProcessImpl()
     : ALLOW_THIS_IN_INITIALIZER_LIST(shared_mem_cache_cleaner_(
           FROM_HERE, base::TimeDelta::FromSeconds(5),
           this, &RenderProcessImpl::ClearTransportDIBCache)),
-      transport_dib_next_sequence_number_(0) {
+      transport_dib_next_sequence_number_(0),
+      enabled_bindings_(0) {
   in_process_plugins_ = InProcessPlugins();
   for (size_t i = 0; i < arraysize(shared_mem_cache_); ++i)
     shared_mem_cache_[i] = NULL;
@@ -98,6 +99,14 @@ bool RenderProcessImpl::InProcessPlugins() {
 #endif
 }
 
+void RenderProcessImpl::AddBindings(int bindings) {
+  enabled_bindings_ |= bindings;
+}
+
+int RenderProcessImpl::GetEnabledBindings() const {
+  return enabled_bindings_;
+}
+
 // -----------------------------------------------------------------------------
 // Platform specific code for dealing with bitmap transport...
 
diff --git a/content/renderer/render_process_impl.h b/content/renderer/render_process_impl.h
index 8761844..ff78b1c 100644
--- a/content/renderer/render_process_impl.h
+++ b/content/renderer/render_process_impl.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -28,6 +28,8 @@ class RenderProcessImpl : public RenderProcess {
       const gfx::Rect& rect) OVERRIDE;
   virtual void ReleaseTransportDIB(TransportDIB* memory) OVERRIDE;
   virtual bool UseInProcessPlugins() const OVERRIDE;
+  virtual void AddBindings(int bindings) OVERRIDE;
+  virtual int GetEnabledBindings() const OVERRIDE;
 
   // Like UseInProcessPlugins(), but called before RenderProcess is created
   // and does not allow overriding by tests. This just checks the command line
@@ -70,6 +72,10 @@ class RenderProcessImpl : public RenderProcess {
 
   bool in_process_plugins_;
 
+  // Bitwise-ORed set of extra bindings that have been enabled anywhere in this
+  // process.  See BindingsPolicy for details.
+  int enabled_bindings_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderProcessImpl);
 };
 
diff --git a/content/renderer/render_view_browsertest.cc b/content/renderer/render_view_browsertest.cc
index 4f8c761..7a9525c 100644
--- a/content/renderer/render_view_browsertest.cc
+++ b/content/renderer/render_view_browsertest.cc
@@ -10,6 +10,7 @@
 #include "content/common/intents_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/native_web_keyboard_event.h"
+#include "content/public/common/bindings_policy.h"
 #include "content/renderer/render_view_impl.h"
 #include "content/test/render_view_test.h"
 #include "net/base/net_errors.h"
@@ -18,6 +19,7 @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebURLError.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebIntentServiceInfo.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebWindowFeatures.h"
 #include "ui/base/keycodes/keyboard_codes.h"
 #include "ui/base/range/range.h"
 #include "ui/gfx/codec/jpeg_codec.h"
@@ -59,6 +61,102 @@ TEST_F(RenderViewImplTest, OnNavStateChanged) {
       ViewHostMsg_UpdateState::ID));
 }
 
+TEST_F(RenderViewImplTest, DecideNavigationPolicy) {
+  // Navigations to normal HTTP URLs can be handled locally.
+  WebKit::WebURLRequest request(GURL("http://foo.com"));
+  WebKit::WebNavigationPolicy policy = view()->decidePolicyForNavigation(
+      GetMainFrame(),
+      request,
+      WebKit::WebNavigationTypeLinkClicked,
+      WebKit::WebNode(),
+      WebKit::WebNavigationPolicyCurrentTab,
+      false);
+  EXPECT_EQ(WebKit::WebNavigationPolicyCurrentTab, policy);
+
+  // Verify that form posts to WebUI URLs will be sent to the browser process.
+  WebKit::WebURLRequest form_request(GURL("chrome://foo"));
+  form_request.setHTTPMethod("POST");
+  policy = view()->decidePolicyForNavigation(
+      GetMainFrame(),
+      form_request,
+      WebKit::WebNavigationTypeFormSubmitted,
+      WebKit::WebNode(),
+      WebKit::WebNavigationPolicyCurrentTab,
+      false);
+  EXPECT_EQ(WebKit::WebNavigationPolicyIgnore, policy);
+
+  // Verify that popup links to WebUI URLs also are sent to browser.
+  WebKit::WebURLRequest popup_request(GURL("chrome://foo"));
+  policy = view()->decidePolicyForNavigation(
+      GetMainFrame(),
+      popup_request,
+      WebKit::WebNavigationTypeLinkClicked,
+      WebKit::WebNode(),
+      WebKit::WebNavigationPolicyNewForegroundTab,
+      false);
+  EXPECT_EQ(WebKit::WebNavigationPolicyIgnore, policy);
+}
+
+TEST_F(RenderViewImplTest, DecideNavigationPolicyForWebUI) {
+  // Enable bindings to simulate a WebUI view.
+  view()->OnAllowBindings(content::BINDINGS_POLICY_WEB_UI);
+
+  // Navigations to normal HTTP URLs will be sent to browser process.
+  WebKit::WebURLRequest request(GURL("http://foo.com"));
+  WebKit::WebNavigationPolicy policy = view()->decidePolicyForNavigation(
+      GetMainFrame(),
+      request,
+      WebKit::WebNavigationTypeLinkClicked,
+      WebKit::WebNode(),
+      WebKit::WebNavigationPolicyCurrentTab,
+      false);
+  EXPECT_EQ(WebKit::WebNavigationPolicyIgnore, policy);
+
+  // Navigations to WebUI URLs will also be sent to browser process.
+  WebKit::WebURLRequest webui_request(GURL("chrome://foo"));
+  policy = view()->decidePolicyForNavigation(
+      GetMainFrame(),
+      webui_request,
+      WebKit::WebNavigationTypeLinkClicked,
+      WebKit::WebNode(),
+      WebKit::WebNavigationPolicyCurrentTab,
+      false);
+  EXPECT_EQ(WebKit::WebNavigationPolicyIgnore, policy);
+
+  // Verify that form posts to data URLs will be sent to the browser process.
+  WebKit::WebURLRequest data_request(GURL("data:text/html,foo"));
+  data_request.setHTTPMethod("POST");
+  policy = view()->decidePolicyForNavigation(
+      GetMainFrame(),
+      data_request,
+      WebKit::WebNavigationTypeFormSubmitted,
+      WebKit::WebNode(),
+      WebKit::WebNavigationPolicyCurrentTab,
+      false);
+  EXPECT_EQ(WebKit::WebNavigationPolicyIgnore, policy);
+
+  // Verify that a popup that creates a view first and then navigates to a
+  // normal HTTP URL will be sent to the browser process, even though the
+  // new view does not have any enabled_bindings_.
+  WebKit::WebURLRequest popup_request(GURL("http://foo.com"));
+  WebKit::WebView* new_web_view = view()->createView(
+      GetMainFrame(), popup_request, WebKit::WebWindowFeatures(), "foo",
+      WebKit::WebNavigationPolicyNewForegroundTab);
+  RenderViewImpl* new_view = RenderViewImpl::FromWebView(new_web_view);
+  policy = new_view->decidePolicyForNavigation(
+      new_web_view->mainFrame(),
+      popup_request,
+      WebKit::WebNavigationTypeLinkClicked,
+      WebKit::WebNode(),
+      WebKit::WebNavigationPolicyNewForegroundTab,
+      false);
+  EXPECT_EQ(WebKit::WebNavigationPolicyIgnore, policy);
+
+  // Clean up after the new view so we don't leak it.
+  new_view->Close();
+  new_view->Release();
+}
+
 // Ensure the RenderViewImpl sends an ACK to a SwapOut request, even if it is
 // already swapped out.  http://crbug.com/93427.
 TEST_F(RenderViewImplTest, SendSwapOutACK) {
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index b1bb198..09ce2cd 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -40,6 +40,7 @@
 #include "content/common/request_extra_data.h"
 #include "content/common/view_messages.h"
 #include "content/public/common/bindings_policy.h"
+#include "content/public/common/content_client.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/context_menu_params.h"
@@ -2316,32 +2317,45 @@ WebNavigationPolicy RenderViewImpl::decidePolicyForNavigation(
 
   // Detect when we're crossing a permission-based boundary (e.g. into or out of
   // an extension or app origin, leaving a WebUI page, etc). We only care about
-  // top-level navigations within the current tab (as opposed to, for example,
-  // opening a new window). But we sometimes navigate to about:blank to clear a
-  // tab, and we want to still allow that.
+  // top-level navigations (not iframes). But we sometimes navigate to
+  // about:blank to clear a tab, and we want to still allow that.
   //
-  // Note: we do this only for GET requests because our mechanism for switching
-  // processes only issues GET requests. In particular, POST requests don't
-  // work, because this mechanism does not preserve form POST data. If it
-  // becomes necessary to support process switching for POST requests, we will
-  // need to send the request's httpBody data up to the browser process, and
-  // issue a special POST navigation in WebKit (via
+  // Note: this is known to break POST submissions when crossing process
+  // boundaries until http://crbug.com/101395 is fixed.  This is better for
+  // security than loading a WebUI, extension or app page in the wrong process.
+  // POST requests don't work because this mechanism does not preserve form
+  // POST data. We will need to send the request's httpBody data up to the
+  // browser process, and issue a special POST navigation in WebKit (via
   // FrameLoader::loadFrameRequest). See ResourceDispatcher and WebURLLoaderImpl
   // for examples of how to send the httpBody data.
   if (!frame->parent() && is_content_initiated &&
-      default_policy == WebKit::WebNavigationPolicyCurrentTab &&
-      request.httpMethod() == "GET" && !url.SchemeIs(chrome::kAboutScheme)) {
+      !url.SchemeIs(chrome::kAboutScheme)) {
     bool send_referrer = false;
+
+    // All navigations to WebUI URLs or within WebUI-enabled RenderProcesses
+    // must be handled by the browser process so that the correct bindings and
+    // data sources can be registered.
+    // Similarly, navigations to view-source URLs or within ViewSource mode
+    // must be handled by the browser process.
+    int cumulative_bindings =
+        RenderProcess::current()->GetEnabledBindings();
     bool should_fork =
-        (enabled_bindings_ & content::BINDINGS_POLICY_WEB_UI) ||
-        frame->isViewSourceModeEnabled() ||
-        url.SchemeIs(chrome::kViewSourceScheme);
+        content::GetContentClient()->HasWebUIScheme(url) ||
+        (cumulative_bindings & content::BINDINGS_POLICY_WEB_UI) ||
+        url.SchemeIs(chrome::kViewSourceScheme) ||
+        frame->isViewSourceModeEnabled();
 
     if (!should_fork) {
       // Give the embedder a chance.
-      bool is_initial_navigation = page_id_ == -1;
-      should_fork = content::GetContentClient()->renderer()->ShouldFork(
-          frame, url, is_initial_navigation, &send_referrer);
+      // For now, we skip this for POST submissions.  This is because
+      // http://crbug.com/101395 is more likely to cause compatibility issues
+      // with hosted apps and extensions than WebUI pages.  We will remove this
+      // check when cross-process POST submissions are supported.
+      if (request.httpMethod() == "GET") {
+        bool is_initial_navigation = page_id_ == -1;
+        should_fork = content::GetContentClient()->renderer()->ShouldFork(
+            frame, url, is_initial_navigation, &send_referrer);
+      }
     }
 
     if (should_fork) {
@@ -4074,6 +4088,9 @@ void RenderViewImpl::OnCSSInsertRequest(const string16& frame_xpath,
 
 void RenderViewImpl::OnAllowBindings(int enabled_bindings_flags) {
   enabled_bindings_ |= enabled_bindings_flags;
+
+  // Keep track of the total bindings accumulated in this process.
+  RenderProcess::current()->AddBindings(enabled_bindings_flags);
 }
 
 void RenderViewImpl::OnSetWebUIProperty(const std::string& name,
diff --git a/content/renderer/render_view_impl.h b/content/renderer/render_view_impl.h
index a104ebe..50997d3 100644
--- a/content/renderer/render_view_impl.h
+++ b/content/renderer/render_view_impl.h
@@ -693,6 +693,7 @@ class RenderViewImpl : public RenderWidget,
   FRIEND_TEST_ALL_PREFIXES(ExternalPopupMenuRemoveTest, RemoveOnChange);
   FRIEND_TEST_ALL_PREFIXES(ExternalPopupMenuTest, NormalCase);
   FRIEND_TEST_ALL_PREFIXES(ExternalPopupMenuTest, ShowPopupThenNavigate);
+  FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest, DecideNavigationPolicyForWebUI);
   FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest,
                            DontIgnoreBackAfterNavEntryLimit);
   FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest, ImeComposition);
@@ -778,7 +779,7 @@ class RenderViewImpl : public RenderWidget,
   // The documentation for these functions should be in
   // render_messages_internal.h for the message that the function is handling.
 
-  void OnAllowBindings(int enabled_bindings_flags);
+  CONTENT_EXPORT void OnAllowBindings(int enabled_bindings_flags);
   void OnAllowScriptToClose(bool script_can_close);
   void OnAsyncFileOpened(base::PlatformFileError error_code,
                          IPC::PlatformFileForTransit file_for_transit,