summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorlfg <lfg@chromium.org>2015-03-19 09:49:53 -0700
committerCommit bot <commit-bot@chromium.org>2015-03-19 16:50:36 +0000
commit052af418cf20a22c485825cd4e2628c4925b5e64 (patch)
tree4e0d30b15a52b44d5d999493d071030a168bff60
parent8824ed5b8ecc1eee3053aee6c80f77a31c32c11f (diff)
downloadchromium_src-052af418cf20a22c485825cd4e2628c4925b5e64.zip
chromium_src-052af418cf20a22c485825cd4e2628c4925b5e64.tar.gz
chromium_src-052af418cf20a22c485825cd4e2628c4925b5e64.tar.bz2
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}
-rw-r--r--chrome/renderer/safe_browsing/phishing_classifier_browsertest.cc2
-rw-r--r--chrome/renderer/safe_browsing/phishing_classifier_delegate_browsertest.cc2
-rw-r--r--chrome/renderer/safe_browsing/phishing_dom_feature_extractor_browsertest.cc2
-rw-r--r--content/browser/accessibility/site_per_process_accessibility_browsertest.cc6
-rw-r--r--content/browser/renderer_host/render_process_host_impl.cc2
-rw-r--r--content/browser/renderer_host/render_widget_helper.cc8
-rw-r--r--content/browser/renderer_host/render_widget_helper.h9
-rw-r--r--content/browser/renderer_host/routing_id_issuer.cc61
-rw-r--r--content/browser/renderer_host/routing_id_issuer.h44
-rw-r--r--content/browser/renderer_host/routing_id_issuer_unittest.cc41
-rw-r--r--content/content_browser.gypi2
-rw-r--r--content/content_tests.gypi3
-rw-r--r--content/public/test/routing_id_mangling_disabler.cc19
-rw-r--r--content/public/test/routing_id_mangling_disabler.h20
-rw-r--r--content/renderer/dom_serializer_browsertest.cc3
-rw-r--r--content/renderer/resource_fetcher_browsertest.cc3
16 files changed, 4 insertions, 223 deletions
diff --git a/chrome/renderer/safe_browsing/phishing_classifier_browsertest.cc b/chrome/renderer/safe_browsing/phishing_classifier_browsertest.cc
index c3e869d..9e7d00d 100644
--- a/chrome/renderer/safe_browsing/phishing_classifier_browsertest.cc
+++ b/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_;
diff --git a/chrome/renderer/safe_browsing/phishing_classifier_delegate_browsertest.cc b/chrome/renderer/safe_browsing/phishing_classifier_delegate_browsertest.cc
index 2623d5d..8f7f29884 100644
--- a/chrome/renderer/safe_browsing/phishing_classifier_delegate_browsertest.cc
+++ b/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) {
diff --git a/chrome/renderer/safe_browsing/phishing_dom_feature_extractor_browsertest.cc b/chrome/renderer/safe_browsing/phishing_dom_feature_extractor_browsertest.cc
index 6392c90..3d1814f 100644
--- a/chrome/renderer/safe_browsing/phishing_dom_feature_extractor_browsertest.cc
+++ b/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_;
diff --git a/content/browser/accessibility/site_per_process_accessibility_browsertest.cc b/content/browser/accessibility/site_per_process_accessibility_browsertest.cc
index 3fd284d..bf9c189 100644
--- a/content/browser/accessibility/site_per_process_accessibility_browsertest.cc
+++ b/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
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index 7ada89c..4152ce2 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/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);
}
diff --git a/content/browser/renderer_host/render_widget_helper.cc b/content/browser/renderer_host/render_widget_helper.cc
index eceb407..e9d83f4 100644
--- a/content/browser/renderer_host/render_widget_helper.cc
+++ b/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
diff --git a/content/browser/renderer_host/render_widget_helper.h b/content/browser/renderer_host/render_widget_helper.h
index e8d7921..7f32035 100644
--- a/content/browser/renderer_host/render_widget_helper.h
+++ b/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_;
diff --git a/content/browser/renderer_host/routing_id_issuer.cc b/content/browser/renderer_host/routing_id_issuer.cc
deleted file mode 100644
index dbcb7f9..0000000
--- a/content/browser/renderer_host/routing_id_issuer.cc
+++ /dev/null
@@ -1,61 +0,0 @@
-// 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
diff --git a/content/browser/renderer_host/routing_id_issuer.h b/content/browser/renderer_host/routing_id_issuer.h
deleted file mode 100644
index 36fc452..0000000
--- a/content/browser/renderer_host/routing_id_issuer.h
+++ /dev/null
@@ -1,44 +0,0 @@
-// 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_
diff --git a/content/browser/renderer_host/routing_id_issuer_unittest.cc b/content/browser/renderer_host/routing_id_issuer_unittest.cc
deleted file mode 100644
index da1e528..0000000
--- a/content/browser/renderer_host/routing_id_issuer_unittest.cc
+++ /dev/null
@@ -1,41 +0,0 @@
-// 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
diff --git a/content/content_browser.gypi b/content/content_browser.gypi
index d213d74..1cd29e3 100644
--- a/content/content_browser.gypi
+++ b/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',
diff --git a/content/content_tests.gypi b/content/content_tests.gypi
index a0a9b4f..f9328bd 100644
--- a/content/content_tests.gypi
+++ b/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',
diff --git a/content/public/test/routing_id_mangling_disabler.cc b/content/public/test/routing_id_mangling_disabler.cc
deleted file mode 100644
index b1e83a1..0000000
--- a/content/public/test/routing_id_mangling_disabler.cc
+++ /dev/null
@@ -1,19 +0,0 @@
-// 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
diff --git a/content/public/test/routing_id_mangling_disabler.h b/content/public/test/routing_id_mangling_disabler.h
deleted file mode 100644
index 5c23174..0000000
--- a/content/public/test/routing_id_mangling_disabler.h
+++ /dev/null
@@ -1,20 +0,0 @@
-// 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_
diff --git a/content/renderer/dom_serializer_browsertest.cc b/content/renderer/dom_serializer_browsertest.cc
index 45dac5b..b175942 100644
--- a/content/renderer/dom_serializer_browsertest.cc
+++ b/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
diff --git a/content/renderer/resource_fetcher_browsertest.cc b/content/renderer/resource_fetcher_browsertest.cc
index fee84ee..92a57a6 100644
--- a/content/renderer/resource_fetcher_browsertest.cc
+++ b/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.