Add RoutingIDIssuer to ensure the correctness of ID to be routed.

This CL introduces RoutingIDIssuer which tracks issued routing
ID per RenderProcessHost and let the Host class verify
routing ids to get routed.

TEST=routing_id_issuer_unitteest.cc
R=jam@chromium.org, sky@chromium.org
TBR=mattm@chromium.org
BUG=381990, 415059

Review URL: https://codereview.chromium.org/643733002

Cr-Commit-Position: refs/heads/master@{#299423}

chrome/renderer/safe_browsing/phishing_classifier_browsertest.cc

@@ -21,6 +21,7 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/renderer/render_view.h"
+#include "content/public/test/routing_id_mangling_disabler.h"
 #include "crypto/sha2.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
@@ -199,6 +200,7 @@ class PhishingClassifierTest : public InProcessBrowserTest {
   scoped_ptr<Scorer> scorer_;
   scoped_ptr<PhishingClassifier> classifier_;
   MockFeatureExtractorClock* clock_;  // Owned by classifier_.
+  content::RoutingIDManglingDisabler mangling_disabler_;
 
   // Features that are in the model.
   const std::string url_tld_token_net_;

chrome/renderer/safe_browsing/phishing_classifier_delegate_browsertest.cc

@@ -26,6 +26,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/renderer/render_view.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/routing_id_mangling_disabler.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
 #include "net/dns/mock_host_resolver.h"
@@ -263,6 +264,7 @@ class PhishingClassifierDelegateTest : public InProcessBrowserTest {
   StrictMock<MockPhishingClassifier>* classifier_;  // Owned by |delegate_|.
   PhishingClassifierDelegate* delegate_;  // Owned by the RenderView.
   scoped_refptr<content::MessageLoopRunner> runner_;
+  content::RoutingIDManglingDisabler mangling_disabler_;
 };
 
 IN_PROC_BROWSER_TEST_F(PhishingClassifierDelegateTest, Navigation) {

chrome/renderer/safe_browsing/phishing_dom_feature_extractor_browsertest.cc

@@ -29,6 +29,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/renderer/render_view.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/routing_id_mangling_disabler.h"
 #include "content/public/test/test_utils.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
@@ -179,6 +180,7 @@ class PhishingDOMFeatureExtractorTest : public InProcessBrowserTest {
   // Any urls not in this map are served a 404 error.
   std::map<std::string, std::string> responses_;
 
+  content::RoutingIDManglingDisabler mangling_disabler_;
   scoped_ptr<net::test_server::EmbeddedTestServer> embedded_test_server_;
   MockFeatureExtractorClock clock_;
   scoped_ptr<PhishingDOMFeatureExtractor> extractor_;

content/browser/accessibility/site_per_process_accessibility_browsertest.cc

@@ -22,6 +22,7 @@
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
+#include "content/public/test/routing_id_mangling_disabler.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "content/test/accessibility_browser_test_utils.h"
@@ -35,6 +36,11 @@ class SitePerProcessAccessibilityBrowserTest
     : public SitePerProcessBrowserTest {
  public:
   SitePerProcessAccessibilityBrowserTest() {}
+
+  // TODO(morrita): This is fishy. This test shouldn't rely on the
+  // abasence of routing_id mangling. Something seems wrong.
+  // See http://crbug.com/422136 for more updates.
+  content::RoutingIDManglingDisabler mangling_disabler_;
 };
 
 // TODO(nasko): try enabling this test on more platforms once

content/browser/renderer_host/render_process_host_impl.cc

@@ -920,6 +920,8 @@ ServiceRegistry* RenderProcessHostImpl::GetServiceRegistry() {
 void RenderProcessHostImpl::AddRoute(
     int32 routing_id,
     IPC::Listener* listener) {
+  DCHECK(widget_helper_->IsRoutingIDProbablyValid(routing_id))
+      << "Found Routing ID conflicts: " << routing_id;
   listeners_.AddWithID(listener, routing_id);
 }
 

content/browser/renderer_host/render_widget_helper.cc

@@ -10,12 +10,13 @@
 #include "base/posix/eintr_wrapper.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
+#include "content/browser/dom_storage/session_storage_namespace_impl.h"
 #include "content/browser/gpu/gpu_process_host_ui_shim.h"
 #include "content/browser/gpu/gpu_surface_tracker.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
-#include "content/browser/dom_storage/session_storage_namespace_impl.h"
+#include "content/browser/renderer_host/routing_id_issuer.h"
 #include "content/common/view_messages.h"
 
 namespace content {
@@ -37,6 +38,7 @@ void AddWidgetHelper(int render_process_id,
 
 RenderWidgetHelper::RenderWidgetHelper()
     : render_process_id_(-1),
+      routing_id_issuer_(RoutingIDIssuer::Create()),
       resource_dispatcher_host_(NULL) {
 }
 
@@ -67,7 +69,11 @@ void RenderWidgetHelper::Init(
 }
 
 int RenderWidgetHelper::GetNextRoutingID() {
-  return next_routing_id_.GetNext() + 1;
+  return routing_id_issuer_->IssueNext();
+}
+
+bool RenderWidgetHelper::IsRoutingIDProbablyValid(int routing_id) const {
+  return routing_id_issuer_->IsProbablyValid(routing_id);
 }
 
 // static

content/browser/renderer_host/render_widget_helper.h

@@ -7,9 +7,9 @@
 
 #include <map>
 
-#include "base/atomic_sequence_num.h"
 #include "base/containers/hash_tables.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/process/process.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
@@ -33,6 +33,7 @@ struct ViewMsg_SwapOut_Params;
 namespace content {
 class GpuProcessHost;
 class ResourceDispatcherHostImpl;
+class RoutingIDIssuer;
 class SessionStorageNamespace;
 
 // Instantiated per RenderProcessHost to provide various optimizations on
@@ -80,6 +81,9 @@ class RenderWidgetHelper
 
   // Gets the next available routing id.  This is thread safe.
   int GetNextRoutingID();
+  // Probablistically verify if the ID is issued by this helper.
+  // Thread safe.
+  bool IsRoutingIDProbablyValid(int routing_id) const;
 
   // IO THREAD ONLY -----------------------------------------------------------
 
@@ -175,8 +179,7 @@ class RenderWidgetHelper
 
   int render_process_id_;
 
-  // The next routing id to use.
-  base::AtomicSequenceNumber next_routing_id_;
+  scoped_ptr<RoutingIDIssuer> routing_id_issuer_;
 
   ResourceDispatcherHostImpl* resource_dispatcher_host_;
 

content/browser/renderer_host/routing_id_issuer.cc

+// 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.
+
+#include "content/browser/renderer_host/routing_id_issuer.h"
+
+namespace content {
+
+namespace {
+
+const int kManglerBits = 3;
+const int kIDBits = 31 - kManglerBits;
+const int kManglerWrap = 1 << kManglerBits;
+const int kIDWrap = 1 << kIDBits;
+const int kIDMask = kIDWrap - 1;
+
+static base::StaticAtomicSequenceNumber g_manglerSequence;
+static int g_disablingIDManglingCount = 0;
+
+int GetNextMangler() {
+  // Does +1 to always gives some mangler.
+  return (g_manglerSequence.GetNext() % (kManglerWrap - 1)) + 1;
+}
+
+}  // namespace
+
+scoped_ptr<RoutingIDIssuer> RoutingIDIssuer::Create() {
+  if (0 < g_disablingIDManglingCount)
+    return make_scoped_ptr(new RoutingIDIssuer(0));
+
+  DCHECK(0 == g_disablingIDManglingCount);
+  return make_scoped_ptr(new RoutingIDIssuer(GetNextMangler()));
+}
+
+scoped_ptr<RoutingIDIssuer> RoutingIDIssuer::CreateWithMangler(int mangler) {
+  return make_scoped_ptr(new RoutingIDIssuer(mangler));
+}
+
+RoutingIDIssuer::RoutingIDIssuer(int mangler) : mangler_(mangler) {
+  DCHECK(0 <= mangler_ && mangler_ < kManglerWrap) << mangler_
+                                                   << ">=" << kManglerWrap;
+}
+
+int RoutingIDIssuer::IssueNext() {
+  return (mangler_ << kIDBits) | ((next_.GetNext() + 1) & kIDMask);
+}
+
+bool RoutingIDIssuer::IsProbablyValid(int issued_id) const {
+  return (mangler_ << kIDBits) == (issued_id & ~kIDMask);
+}
+
+void RoutingIDIssuer::DisableIDMangling() {
+  g_disablingIDManglingCount++;
+}
+
+void RoutingIDIssuer::EnableIDMangling() {
+  g_disablingIDManglingCount--;
+  DCHECK(0 <= g_disablingIDManglingCount);
+}
+
+}  // namespace content

content/browser/renderer_host/routing_id_issuer.h

+// Copyright 2014 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.
+
+#ifndef CONTENT_BROWSER_RENDERER_HOST_ROUTING_ID_ISSUER_H_
+#define CONTENT_BROWSER_RENDERER_HOST_ROUTING_ID_ISSUER_H_
+
+#include "base/atomic_sequence_num.h"
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/synchronization/lock.h"
+#include "content/common/content_export.h"
+#include "ipc/ipc_message.h"
+
+namespace content {
+
+// A debug facility for tightening the routing ID usage across multiple
+// RenderProcessHost. The tracked ID is used by RenderProcessHost
+// to verify if the given ID is correct.
+class CONTENT_EXPORT RoutingIDIssuer {
+ public:
+  static scoped_ptr<RoutingIDIssuer> Create();
+  static scoped_ptr<RoutingIDIssuer> CreateWithMangler(int mangler);
+
+  int IssueNext();
+  bool IsProbablyValid(int issued_id) const;
+
+ private:
+  friend class RoutingIDManglingDisabler;
+
+  static void DisableIDMangling();
+  static void EnableIDMangling();
+
+  explicit RoutingIDIssuer(int mangler);
+
+  base::AtomicSequenceNumber next_;
+  const int mangler_;
+
+  DISALLOW_COPY_AND_ASSIGN(RoutingIDIssuer);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_RENDERER_HOST_ROUTING_ID_ISSUER_H_

content/browser/renderer_host/routing_id_issuer_unittest.cc

+// Copyright 2014 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.
+
+#include "content/browser/renderer_host/routing_id_issuer.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+namespace {
+
+const int kMangler = 5;
+
+class RoutingIDIssuerTest : public testing::Test {};
+
+TEST_F(RoutingIDIssuerTest, IssueNext) {
+  scoped_ptr<RoutingIDIssuer> target =
+      RoutingIDIssuer::CreateWithMangler(kMangler);
+  int id0 = target->IssueNext();
+  int id1 = target->IssueNext();
+  EXPECT_EQ(id0 + 1, id1);
+}
+
+TEST_F(RoutingIDIssuerTest, IsProbablyValid) {
+  scoped_ptr<RoutingIDIssuer> proper =
+      RoutingIDIssuer::CreateWithMangler(kMangler);
+  scoped_ptr<RoutingIDIssuer> foreign =
+      RoutingIDIssuer::CreateWithMangler(kMangler + 1);
+
+  EXPECT_FALSE(proper->IsProbablyValid(kMangler));
+
+  int proper_id = proper->IssueNext();
+  int foreign_id = foreign->IssueNext();
+  EXPECT_TRUE(proper->IsProbablyValid(proper_id));
+  EXPECT_TRUE(foreign->IsProbablyValid(foreign_id));
+  EXPECT_FALSE(proper->IsProbablyValid(foreign_id));
+  EXPECT_FALSE(foreign->IsProbablyValid(proper_id));
+}
+
+}  // namespace
+}  // namespace contnet

content/content_browser.gypi

@@ -1115,6 +1115,8 @@
       'browser/renderer_host/render_widget_resize_helper.h',
       'browser/renderer_host/renderer_frame_manager.cc',
       'browser/renderer_host/renderer_frame_manager.h',
+      'browser/renderer_host/routing_id_issuer.cc',
+      'browser/renderer_host/routing_id_issuer.h',
       'browser/renderer_host/sandbox_ipc_linux.cc',
       'browser/renderer_host/sandbox_ipc_linux.h',
       'browser/renderer_host/software_frame_manager.cc',

content/content_tests.gypi

@@ -54,6 +54,8 @@
       'public/test/render_view_test.h',
       'public/test/render_widget_test.cc',
       'public/test/render_widget_test.h',
+      'public/test/routing_id_mangling_disabler.cc',
+      'public/test/routing_id_mangling_disabler.h',
       'public/test/sandbox_file_system_test_helper.cc',
       'public/test/sandbox_file_system_test_helper.h',
       'public/test/test_browser_context.cc',
@@ -583,6 +585,7 @@
         'browser/renderer_host/render_widget_host_view_base_unittest.cc',
         'browser/renderer_host/render_widget_host_view_mac_editcommand_helper_unittest.mm',
         'browser/renderer_host/render_widget_host_view_mac_unittest.mm',
+        'browser/renderer_host/routing_id_issuer_unittest.cc',
         'browser/renderer_host/software_frame_manager_unittest.cc',
         'browser/renderer_host/text_input_client_mac_unittest.mm',
         'browser/renderer_host/web_input_event_aura_unittest.cc',

content/public/test/routing_id_mangling_disabler.cc

+// Copyright 2014 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.
+
+#include "content/public/test/routing_id_mangling_disabler.h"
+
+#include "content/browser/renderer_host/routing_id_issuer.h"
+
+namespace content {
+
+RoutingIDManglingDisabler::RoutingIDManglingDisabler() {
+  RoutingIDIssuer::DisableIDMangling();
+}
+
+RoutingIDManglingDisabler::~RoutingIDManglingDisabler() {
+  RoutingIDIssuer::EnableIDMangling();
+}
+
+}  // namespace content

content/public/test/routing_id_mangling_disabler.h

+// Copyright 2014 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.
+
+#ifndef CONTENT_PUBLIC_TEST_ROUTIND_ID_MANGLING_DISABLER_H_
+#define CONTENT_PUBLIC_TEST_ROUTIND_ID_MANGLING_DISABLER_H_
+
+namespace content {
+
+// Some tests unfortunately assume that the routing ID starts from
+// one. This class temporarily satisfies such an assumption.
+class RoutingIDManglingDisabler {
+ public:
+  RoutingIDManglingDisabler();
+  ~RoutingIDManglingDisabler();
+};
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_TEST_ROUTIND_ID_MANGLING_DISABLER_H_

content/renderer/dom_serializer_browsertest.cc

@@ -15,6 +15,7 @@
 #include "content/public/renderer/render_view_observer.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
+#include "content/public/test/routing_id_mangling_disabler.h"
 #include "content/public/test/test_utils.h"
 #include "content/renderer/savable_resources.h"
 #include "content/shell/browser/shell.h"
@@ -774,6 +775,8 @@ class DomSerializerTests : public ContentBrowserTest,
   // The local_directory_name_ is dummy relative path of directory which
   // contain all saved auxiliary files included all sub frames and resources.
   const base::FilePath local_directory_name_;
+
+  content::RoutingIDManglingDisabler mangling_disabler_;
 };
 
 // If original contents have document type, the serialized contents also have

content/renderer/resource_fetcher_browsertest.cc

@@ -15,6 +15,7 @@
 #include "content/public/renderer/render_view.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
+#include "content/public/test/routing_id_mangling_disabler.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "third_party/WebKit/public/platform/WebURLResponse.h"
@@ -284,6 +285,8 @@ class ResourceFetcherTests : public ContentBrowserTest {
     EXPECT_EQ(delegate->response().httpStatusCode(), 200);
     EXPECT_EQ(kHeader, delegate->data());
   }
+
+  content::RoutingIDManglingDisabler routing_id_mangling_disabler_;
 };
 
 #if defined(OS_ANDROID)