Revert of Add RoutingIDIssuer to help track routing id issues. (patchset #4...

Revert of Add RoutingIDIssuer to help track routing id issues. (patchset #4 id:60001 of https://codereview.chromium.org/999193003/)

Reason for revert:
We've already collected enough data to triage this bug.

Original issue's description:
> This is a re-land of https://codereview.chromium.org/643733002.
>
> This CL adds a RoutingIDIssuer that verifies that the routing IDs are being sent to the correct process.
>
> BUG=464633
> TBR=sky@chromium.org
>
> Committed: https://crrev.com/5ae65efdfd51de794fb544195fad271dd90ff0b1
> Cr-Commit-Position: refs/heads/master@{#320207}

TBR=nasko@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=464633

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

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

chrome/renderer/safe_browsing/phishing_classifier_browsertest.cc

@@ -21,7 +21,6 @@
 #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"
@@ -200,7 +199,6 @@ 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,7 +26,6 @@
 #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"
@@ -264,7 +263,6 @@ 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,7 +29,6 @@
 #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"
@@ -180,7 +179,6 @@ 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

@@ -23,7 +23,6 @@
 #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"
@@ -38,11 +37,6 @@ 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_;
 };
 
 // Utility function to determine if an accessibility tree has finished loading

content/browser/renderer_host/render_process_host_impl.cc

@@ -1017,8 +1017,6 @@ void RenderProcessHostImpl::AddRoute(
     IPC::Listener* listener) {
   CHECK(!listeners_.Lookup(routing_id))
       << "Found Routing ID Conflict: " << routing_id;
-  CHECK(widget_helper_->IsRoutingIDProbablyValid(routing_id))
-      << "Found Routing ID conflicts: " << routing_id;
   listeners_.AddWithID(listener, routing_id);
 }
 

content/browser/renderer_host/render_widget_helper.cc

@@ -16,7 +16,6 @@
 #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/renderer_host/routing_id_issuer.h"
 #include "content/common/view_messages.h"
 
 namespace content {
@@ -38,7 +37,6 @@ void AddWidgetHelper(int render_process_id,
 
 RenderWidgetHelper::RenderWidgetHelper()
     : render_process_id_(-1),
-      routing_id_issuer_(RoutingIDIssuer::Create()),
       resource_dispatcher_host_(NULL) {
 }
 
@@ -65,11 +63,7 @@ void RenderWidgetHelper::Init(
 }
 
 int RenderWidgetHelper::GetNextRoutingID() {
-  return routing_id_issuer_->IssueNext();
-}
-
-bool RenderWidgetHelper::IsRoutingIDProbablyValid(int routing_id) const {
-  return routing_id_issuer_->IsProbablyValid(routing_id);
+  return next_routing_id_.GetNext() + 1;
 }
 
 // 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"
@@ -32,7 +32,6 @@ struct ViewMsg_SwapOut_Params;
 namespace content {
 class GpuProcessHost;
 class ResourceDispatcherHostImpl;
-class RoutingIDIssuer;
 class SessionStorageNamespace;
 
 // Instantiated per RenderProcessHost to provide various optimizations on
@@ -80,9 +79,6 @@ 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 -----------------------------------------------------------
 
@@ -154,7 +150,8 @@ class RenderWidgetHelper
 
   int render_process_id_;
 
-  scoped_ptr<RoutingIDIssuer> routing_id_issuer_;
+  // The next routing id to use.
+  base::AtomicSequenceNumber next_routing_id_;
 
   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 = 6;
-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

@@ -1218,8 +1218,6 @@
       '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/text_input_client_mac.h',

content/content_tests.gypi

@@ -67,8 +67,6 @@
       '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',
@@ -523,7 +521,6 @@
       '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/text_input_client_mac_unittest.mm',
       'browser/renderer_host/web_input_event_aura_unittest.cc',
       'browser/renderer_host/websocket_dispatcher_host_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,7 +15,6 @@
 #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"
@@ -775,8 +774,6 @@ 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,7 +15,6 @@
 #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"
@@ -285,8 +284,6 @@ class ResourceFetcherTests : public ContentBrowserTest {
     EXPECT_EQ(delegate->response().httpStatusCode(), 200);
     EXPECT_EQ(kHeader, delegate->data());
   }
-
-  content::RoutingIDManglingDisabler routing_id_mangling_disabler_;
 };
 
 // Test a fetch from the test server.