Skip to content

Commit

Permalink
Rename two checkers to "sequence checkers"
Browse files Browse the repository at this point in the history
It's more descriptive and more inclusive.

Bug: 842296
Tbr: michaelbai@chromium.org
Tbr: battre@chromium.org
Change-Id: Icbc901c25c65cdfe5c1da70819d02f7b9f56e6a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2292808
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#787367}
  • Loading branch information
Avi Drissman authored and Commit Bot committed Jul 10, 2020
1 parent 99cd974 commit 81ac39b
Show file tree
Hide file tree
Showing 15 changed files with 123 additions and 122 deletions.
Expand Up @@ -289,8 +289,8 @@ class ContentAutofillDriverTest : public content::RenderViewHostTestHarness {
public:
void SetUp() override {
content::RenderViewHostTestHarness::SetUp();
// This needed to keep the WebContentsObserverSanityChecker checks happy for
// when AppendChild is called.
// This needed to keep the WebContentsObserverSequenceChecker checks happy
// for when AppendChild is called.
NavigateAndCommit(GURL("about:blank"));

test_autofill_client_.reset(new MockAutofillClient());
Expand Down
Expand Up @@ -160,8 +160,8 @@ class ContentCaptureReceiverTest : public content::RenderViewHostTestHarness {
content_capture_receiver_manager_helper_ =
static_cast<ContentCaptureReceiverManagerHelper*>(
ContentCaptureReceiverManager::FromWebContents(web_contents()));
// This needed to keep the WebContentsObserverSanityChecker checks happy for
// when AppendChild is called.
// This needed to keep the WebContentsObserverSequenceChecker checks happy
// for when AppendChild is called.
NavigateAndCommit(GURL(kMainFrameUrl));
content_capture_sender_ = std::make_unique<FakeContentCaptureSender>();
main_frame_ = web_contents()->GetMainFrame();
Expand Down Expand Up @@ -644,8 +644,8 @@ class ContentCaptureReceiverMultipleFrameTest
void SetUp() override {
// Setup multiple frames before creates ContentCaptureReceiverManager.
content::RenderViewHostTestHarness::SetUp();
// This needed to keep the WebContentsObserverSanityChecker checks happy for
// when AppendChild is called.
// This needed to keep the WebContentsObserverSequenceChecker checks happy
// for when AppendChild is called.
NavigateAndCommit(GURL("about:blank"));
content::RenderFrameHostTester::For(web_contents()->GetMainFrame())
->AppendChild("child");
Expand Down
2 changes: 1 addition & 1 deletion content/browser/frame_host/navigation_request_unittest.cc
Expand Up @@ -72,7 +72,7 @@ class NavigationRequestTest : public RenderViewHostImplTestHarness {

void TearDown() override {
// Release the |request_| before destroying the WebContents, to match
// the WebContentsObserverSanityChecker expectations.
// the WebContentsObserverSequenceChecker expectations.
request_.reset();
RenderViewHostImplTestHarness::TearDown();
}
Expand Down
Expand Up @@ -52,8 +52,8 @@ class PresentationServiceImplTest : public RenderViewHostImplTestHarness {

void SetUp() override {
RenderViewHostImplTestHarness::SetUp();
// This needed to keep the WebContentsObserverSanityChecker checks happy for
// when AppendChild is called.
// This needed to keep the WebContentsObserverSequenceChecker checks happy
// for when AppendChild is called.
NavigateAndCommit(GURL("about:blank"));

EXPECT_CALL(mock_delegate_, AddObserver(_, _, _)).Times(1);
Expand Down
2 changes: 1 addition & 1 deletion content/browser/web_contents/web_contents_impl.h
Expand Up @@ -2082,7 +2082,7 @@ class CONTENT_EXPORT WebContentsImpl::FriendWrapper {
private:
friend class TestNavigationObserver;
friend class WebContentsAddedObserver;
friend class ContentBrowserSanityChecker;
friend class ContentBrowserSequenceChecker;

FriendWrapper(); // Not instantiable.

Expand Down
4 changes: 2 additions & 2 deletions content/public/test/browser_test_base.cc
Expand Up @@ -56,7 +56,7 @@
#include "content/public/test/no_renderer_crashes_assertion.h"
#include "content/public/test/test_launcher.h"
#include "content/public/test/test_utils.h"
#include "content/test/content_browser_sanity_checker.h"
#include "content/test/content_browser_sequence_checker.h"
#include "gpu/command_buffer/service/gpu_switches.h"
#include "gpu/config/gpu_switches.h"
#include "media/base/media_switches.h"
Expand Down Expand Up @@ -325,7 +325,7 @@ void BrowserTestBase::SetUp() {
if (!allow_network_access_to_host_resolutions_)
test_host_resolver_ = std::make_unique<TestHostResolver>();

ContentBrowserSanityChecker scoped_enable_sanity_checks;
ContentBrowserSequenceChecker scoped_enable_sequence_checks;

SetUpInProcessBrowserTestFixture();

Expand Down
4 changes: 2 additions & 2 deletions content/public/test/test_renderer_host.cc
Expand Up @@ -27,7 +27,7 @@
#include "content/public/test/mock_render_process_host.h"
#include "content/public/test/navigation_simulator.h"
#include "content/public/test/test_browser_context.h"
#include "content/test/content_browser_sanity_checker.h"
#include "content/test/content_browser_sequence_checker.h"
#include "content/test/test_navigation_url_loader_factory.h"
#include "content/test/test_render_frame_host.h"
#include "content/test/test_render_frame_host_factory.h"
Expand Down Expand Up @@ -244,7 +244,7 @@ void RenderViewHostTestHarness::SetUp() {
aura_test_helper_->SetUp();
#endif

sanity_checker_ = std::make_unique<ContentBrowserSanityChecker>();
sequence_checker_ = std::make_unique<ContentBrowserSequenceChecker>();

#if !defined(OS_ANDROID)
network_change_notifier_ = net::test::MockNetworkChangeNotifier::Create();
Expand Down
4 changes: 2 additions & 2 deletions content/public/test/test_renderer_host.h
Expand Up @@ -49,7 +49,7 @@ class ScopedOleInitializer;
namespace content {

class BrowserContext;
class ContentBrowserSanityChecker;
class ContentBrowserSequenceChecker;
class MockRenderProcessHost;
class MockRenderProcessHostFactory;
class NavigationController;
Expand Down Expand Up @@ -279,7 +279,7 @@ class RenderViewHostTestHarness : public testing::Test {

std::unique_ptr<BrowserTaskEnvironment> task_environment_;

std::unique_ptr<ContentBrowserSanityChecker> sanity_checker_;
std::unique_ptr<ContentBrowserSequenceChecker> sequence_checker_;

// TODO(crbug.com/1011275): This is a temporary work around to fix flakiness
// on tests. The default behavior of the network stack is to allocate a
Expand Down
8 changes: 4 additions & 4 deletions content/test/BUILD.gn
Expand Up @@ -243,8 +243,8 @@ jumbo_static_library("test_support") {
"../public/test/web_contents_tester.h",
"../public/test/web_ui_browsertest_util.cc",
"../public/test/web_ui_browsertest_util.h",
"content_browser_sanity_checker.cc",
"content_browser_sanity_checker.h",
"content_browser_sequence_checker.cc",
"content_browser_sequence_checker.h",
"content_test_suite.cc",
"content_test_suite.h",
"did_commit_navigation_interceptor.cc",
Expand Down Expand Up @@ -347,8 +347,8 @@ jumbo_static_library("test_support") {
"test_web_contents.cc",
"test_web_contents.h",
"test_web_contents_factory.cc",
"web_contents_observer_sanity_checker.cc",
"web_contents_observer_sanity_checker.h",
"web_contents_observer_sequence_checker.cc",
"web_contents_observer_sequence_checker.h",
"web_gesture_curve_mock.cc",
"web_gesture_curve_mock.h",
]
Expand Down
40 changes: 0 additions & 40 deletions content/test/content_browser_sanity_checker.cc

This file was deleted.

40 changes: 40 additions & 0 deletions content/test/content_browser_sequence_checker.cc
@@ -0,0 +1,40 @@
// 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/test/content_browser_sequence_checker.h"

#include "base/bind.h"
#include "content/browser/web_contents/web_contents_impl.h"
#include "content/test/web_contents_observer_sequence_checker.h"

namespace content {

namespace {
bool g_sequence_checks_already_enabled = false;
}

ContentBrowserSequenceChecker::ContentBrowserSequenceChecker() {
CHECK(!g_sequence_checks_already_enabled)
<< "Tried to enable ContentBrowserSequenceChecker, but it's already been "
<< "enabled.";
g_sequence_checks_already_enabled = true;

creation_hook_ =
base::BindRepeating(&ContentBrowserSequenceChecker::OnWebContentsCreated,
base::Unretained(this));
WebContentsImpl::FriendWrapper::AddCreatedCallbackForTesting(creation_hook_);
}

ContentBrowserSequenceChecker::~ContentBrowserSequenceChecker() {
WebContentsImpl::FriendWrapper::RemoveCreatedCallbackForTesting(
creation_hook_);
g_sequence_checks_already_enabled = false;
}

void ContentBrowserSequenceChecker::OnWebContentsCreated(
WebContents* web_contents) {
WebContentsObserverSequenceChecker::Enable(web_contents);
}

} // namespace content
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CONTENT_TEST_CONTENT_BROWSER_SANITY_CHECKER_H_
#define CONTENT_TEST_CONTENT_BROWSER_SANITY_CHECKER_H_
#ifndef CONTENT_TEST_CONTENT_BROWSER_SEQUENCE_CHECKER_H_
#define CONTENT_TEST_CONTENT_BROWSER_SEQUENCE_CHECKER_H_

#include "base/callback.h"
#include "base/macros.h"
Expand All @@ -12,8 +12,8 @@ namespace content {

class WebContents;

// While an instance of this class exists, a bunch of sanity-checks are enabled
// to validate the correctness of the browser side of the content layer
// While an instance of this class exists, a bunch of sequence checks are
// enabled to validate the correctness of the browser side of the content layer
// implementation. It's good to enable these in both unit tests and browser
// tests, as a means of detecting bugs in the implementation of the content api.
//
Expand All @@ -23,20 +23,20 @@ class WebContents;
// typically need to enable them.
//
// For the nuts and bolts of what the checks enforce, see the implementation.
class ContentBrowserSanityChecker {
class ContentBrowserSequenceChecker {
public:
ContentBrowserSanityChecker();
~ContentBrowserSanityChecker();
ContentBrowserSequenceChecker();
~ContentBrowserSequenceChecker();

private:
void OnWebContentsCreated(WebContents* web_contents);

// The callback needs to be cached so that it can be unregistered.
base::RepeatingCallback<void(WebContents*)> creation_hook_;

DISALLOW_COPY_AND_ASSIGN(ContentBrowserSanityChecker);
DISALLOW_COPY_AND_ASSIGN(ContentBrowserSequenceChecker);
};

} // namespace content

#endif // CONTENT_TEST_CONTENT_BROWSER_SANITY_CHECKER_H_
#endif // CONTENT_TEST_CONTENT_BROWSER_SEQUENCE_CHECKER_H_

0 comments on commit 81ac39b

Please sign in to comment.