Skip to content

Commit

Permalink
Add helper class to register/unregister WebClient.
Browse files Browse the repository at this point in the history
ScopedTestingWebClient register a WebClient at initialisation and
unregister at destruction, restoring the previous WebClient. This
class also assert if the registered WebClient has been changed by
the time the destructor is reached.

Port all unit tests to use ScopedTestingWebClient when installing
a WebClient instead of using SetWebClient to avoid mistakes.

BUG=None

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

Cr-Commit-Position: refs/heads/master@{#365840}
  • Loading branch information
sdefresne authored and Commit bot committed Dec 17, 2015
1 parent b7e1bec commit 18f952a
Show file tree
Hide file tree
Showing 16 changed files with 150 additions and 84 deletions.
19 changes: 15 additions & 4 deletions ios/chrome/test/ios_chrome_unit_test_suite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "ios/chrome/browser/chrome_paths.h"
#include "ios/chrome/browser/chrome_url_constants.h"
#include "ios/chrome/test/testing_application_context.h"
#include "ios/public/provider/chrome/browser/chrome_browser_provider.h"
#include "ios/public/test/test_chrome_provider_initializer.h"
#include "ios/web/public/web_client.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand All @@ -24,18 +25,28 @@ class IOSChromeUnitTestSuiteInitializer
~IOSChromeUnitTestSuiteInitializer() override {}

void OnTestStart(const testing::TestInfo& test_info) override {
web_client_.reset(new web::WebClient());
DCHECK(!web_client_);
web_client_.reset(new web::WebClient);
web::SetWebClient(web_client_.get());

DCHECK(!ios::GetChromeBrowserProvider());
test_ios_chrome_provider_initializer_.reset(
new ios::TestChromeProviderInitializer());

DCHECK(!GetApplicationContext());
application_context_.reset(new TestingApplicationContext);
}

void OnTestEnd(const testing::TestInfo& test_info) override {
web_client_.reset();
web::SetWebClient(nullptr);
test_ios_chrome_provider_initializer_.reset();
DCHECK_EQ(GetApplicationContext(), application_context_.get());
application_context_.reset();

test_ios_chrome_provider_initializer_.reset();
DCHECK(!ios::GetChromeBrowserProvider());

DCHECK_EQ(web::GetWebClient(), web_client_.get());
web::SetWebClient(nullptr);
web_client_.reset();
}

private:
Expand Down
2 changes: 2 additions & 0 deletions ios/web/ios_web.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,8 @@
'public/test/response_providers/response_provider.h',
'public/test/response_providers/string_response_provider.h',
'public/test/response_providers/string_response_provider.mm',
'public/test/scoped_testing_web_client.cc',
'public/test/scoped_testing_web_client.h',
'public/test/test_browser_state.cc',
'public/test/test_browser_state.h',
'public/test/test_web_client.h',
Expand Down
6 changes: 3 additions & 3 deletions ios/web/net/crw_url_verifying_protocol_handler_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "ios/web/net/crw_url_verifying_protocol_handler.h"

#include "base/memory/scoped_ptr.h"
#include "ios/web/public/test/scoped_testing_web_client.h"
#import "ios/web/public/test/test_web_client.h"
#include "ios/web/public/web_client.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand All @@ -13,10 +14,9 @@

// TODO(shreyasv): See if this can use the WebTest test fixture.
TEST(CRWURLVerifyingProtocolHandlerTest, NonLazyInitializer) {
scoped_ptr<web::TestWebClient> test_web_client(new web::TestWebClient());
web::SetWebClient(test_web_client.get());
web::ScopedTestingWebClient web_client(
make_scoped_ptr(new web::TestWebClient));
EXPECT_TRUE([CRWURLVerifyingProtocolHandler preInitialize]);
web::SetWebClient(nullptr);
}

} // namespace
13 changes: 3 additions & 10 deletions ios/web/net/web_http_protocol_handler_delegate_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/memory/scoped_ptr.h"
#include "base/message_loop/message_loop.h"
#include "base/thread_task_runner_handle.h"
#include "ios/web/public/test/scoped_testing_web_client.h"
#include "ios/web/public/web_client.h"
#include "net/url_request/url_request_test_util.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -48,21 +49,13 @@ bool IsAppSpecificURL(const GURL& url) const override {
: context_getter_(new net::TestURLRequestContextGetter(
base::ThreadTaskRunnerHandle::Get())),
delegate_(new WebHTTPProtocolHandlerDelegate(context_getter_.get())),
original_web_client_(GetWebClient()),
web_client_(new AppSpecificURLTestWebClient) {
SetWebClient(web_client_.get());
}

~WebHTTPProtocolHandlerDelegateTest() override {
SetWebClient(original_web_client_);
}
web_client_(make_scoped_ptr(new AppSpecificURLTestWebClient)) {}

protected:
base::MessageLoop message_loop_;
scoped_refptr<net::URLRequestContextGetter> context_getter_;
scoped_ptr<WebHTTPProtocolHandlerDelegate> delegate_;
WebClient* original_web_client_; // Stash the web client, Weak.
scoped_ptr<AppSpecificURLTestWebClient> web_client_;
web::ScopedTestingWebClient web_client_;
};

} // namespace
Expand Down
26 changes: 26 additions & 0 deletions ios/web/public/test/scoped_testing_web_client.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2015 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 "ios/web/public/test/scoped_testing_web_client.h"

#include "base/logging.h"
#include "ios/web/public/web_client.h"

namespace web {

ScopedTestingWebClient::ScopedTestingWebClient(scoped_ptr<WebClient> web_client)
: web_client_(std::move(web_client)), original_web_client_(GetWebClient()) {
SetWebClient(web_client_.get());
}

ScopedTestingWebClient::~ScopedTestingWebClient() {
DCHECK_EQ(GetWebClient(), web_client_.get());
SetWebClient(original_web_client_);
}

WebClient* ScopedTestingWebClient::Get() {
return web_client_.get();
}

} // namespace web
30 changes: 30 additions & 0 deletions ios/web/public/test/scoped_testing_web_client.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2015 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 IOS_WEB_PUBLIC_TEST_SCOPED_TESTING_WEB_CLIENT_H_
#define IOS_WEB_PUBLIC_TEST_SCOPED_TESTING_WEB_CLIENT_H_

#include "base/macros.h"
#include "base/memory/scoped_ptr.h"

namespace web {

class WebClient;

// Helper class to register a WebClient during unit testing.
class ScopedTestingWebClient {
public:
explicit ScopedTestingWebClient(scoped_ptr<WebClient> web_client);
~ScopedTestingWebClient();

WebClient* Get();

private:
scoped_ptr<WebClient> web_client_;
WebClient* original_web_client_;
};

} // namespace web

#endif // IOS_WEB_PUBLIC_TEST_SCOPED_TESTING_WEB_CLIENT_H_
5 changes: 3 additions & 2 deletions ios/web/test/web_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#import "base/mac/scoped_nsobject.h"
#include "base/message_loop/message_loop.h"
#include "ios/web/public/test/scoped_testing_web_client.h"
#include "ios/web/public/test/test_browser_state.h"
#import "ios/web/public/test/test_web_client.h"
#include "ios/web/public/test/test_web_thread_bundle.h"
Expand Down Expand Up @@ -43,14 +44,14 @@ class WebTest : public PlatformTest {
void TearDown() override;

// Returns the WebClient that is used for testing.
TestWebClient* GetWebClient() { return &client_; }
TestWebClient* GetWebClient();

// Returns the BrowserState that is used for testing.
BrowserState* GetBrowserState() { return &browser_state_; }

private:
// The WebClient used in tests.
TestWebClient client_;
ScopedTestingWebClient web_client_;
// The threads used for testing.
web::TestWebThreadBundle thread_bundle_;
// The browser state used in tests.
Expand Down
8 changes: 5 additions & 3 deletions ios/web/test/web_test.mm
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,23 @@ - (BOOL)webController:(CRWWebController*)webController

#pragma mark -

WebTest::WebTest() {}
WebTest::WebTest() : web_client_(make_scoped_ptr(new TestWebClient)) {}
WebTest::~WebTest() {}

void WebTest::SetUp() {
PlatformTest::SetUp();
web::SetWebClient(&client_);
BrowserState::GetActiveStateManager(&browser_state_)->SetActive(true);
}

void WebTest::TearDown() {
BrowserState::GetActiveStateManager(&browser_state_)->SetActive(false);
web::SetWebClient(nullptr);
PlatformTest::TearDown();
}

TestWebClient* WebTest::GetWebClient() {
return static_cast<TestWebClient*>(web_client_.Get());
}

#pragma mark -

WebTestWithWebController::WebTestWithWebController() {}
Expand Down
18 changes: 11 additions & 7 deletions ios/web/ui_web_view_util_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#import "ios/web/ui_web_view_util.h"

#include "ios/web/public/test/scoped_testing_web_client.h"
#import "ios/web/public/test/test_web_client.h"
#include "testing/gtest_mac.h"
#include "testing/platform_test.h"
Expand All @@ -16,20 +17,23 @@
}

class UIWebViewUtilTest : public PlatformTest {
public:
UIWebViewUtilTest() : web_client_(make_scoped_ptr(new web::TestWebClient)) {}

protected:
void SetUp() override {
PlatformTest::SetUp();
test_web_client_.SetUserAgent("DesktopUA", true);
test_web_client_.SetUserAgent("RegularUA", false);
web::SetWebClient(&test_web_client_);
test_web_client()->SetUserAgent("DesktopUA", true);
test_web_client()->SetUserAgent("RegularUA", false);
}
void TearDown() override {
web::SetWebClient(nullptr);
PlatformTest::TearDown();

web::TestWebClient* test_web_client() {
return static_cast<web::TestWebClient*>(web_client_.Get());
}

private:
// WebClient that returns test user agent.
web::TestWebClient test_web_client_;
web::ScopedTestingWebClient web_client_;
};

// Tests registration of a non-desktop user agent.
Expand Down
12 changes: 6 additions & 6 deletions ios/web/web_state/js/crw_js_early_script_manager_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "base/mac/scoped_nsobject.h"
#import "ios/web/public/test/crw_test_js_injection_receiver.h"
#include "ios/web/public/test/scoped_testing_web_client.h"
#include "ios/web/public/web_client.h"
#import "ios/web/web_state/js/page_script_util.h"
#import "testing/gtest_mac.h"
Expand All @@ -15,24 +16,23 @@
namespace {

class CRWJSEarlyScriptManagerTest : public PlatformTest {
public:
CRWJSEarlyScriptManagerTest() : web_client_(make_scoped_ptr(new WebClient)) {}

protected:
void SetUp() override {
PlatformTest::SetUp();
SetWebClient(&web_client_);
receiver_.reset([[CRWTestJSInjectionReceiver alloc] init]);
earlyScriptManager_.reset(static_cast<CRWJSEarlyScriptManager*>(
[[receiver_ instanceOfClass:[CRWJSEarlyScriptManager class]] retain]));
}
void TearDown() override {
SetWebClient(nullptr);
PlatformTest::TearDown();
}

// Required for CRWJSEarlyScriptManager creation.
base::scoped_nsobject<CRWTestJSInjectionReceiver> receiver_;
// Testable CRWJSEarlyScriptManager.
base::scoped_nsobject<CRWJSEarlyScriptManager> earlyScriptManager_;
// WebClient required for getting early page script.
WebClient web_client_;
ScopedTestingWebClient web_client_;
};

// Tests that CRWJSEarlyScriptManager's content is the same as returned by
Expand Down
12 changes: 6 additions & 6 deletions ios/web/web_state/js/crw_js_window_id_manager_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,31 @@
#include "base/mac/scoped_nsobject.h"
#import "ios/web/public/test/crw_test_js_injection_receiver.h"
#import "ios/web/public/test/js_test_util.h"
#include "ios/web/public/test/scoped_testing_web_client.h"
#include "ios/web/public/web_client.h"
#import "testing/gtest_mac.h"
#include "testing/platform_test.h"

namespace {

class JSWindowIDManagerTest : public PlatformTest {
public:
JSWindowIDManagerTest() : web_client_(make_scoped_ptr(new web::WebClient)) {}

protected:
void SetUp() override {
PlatformTest::SetUp();
receiver_.reset([[CRWTestJSInjectionReceiver alloc] init]);
manager_.reset([[CRWJSWindowIdManager alloc] initWithReceiver:receiver_]);
web::SetWebClient(&web_client_);
}
void TearDown() override {
web::SetWebClient(nullptr);
PlatformTest::TearDown();
}

// Required for CRWJSWindowIdManager creation.
base::scoped_nsobject<CRWTestJSInjectionReceiver> receiver_;
// Testable CRWJSWindowIdManager.
base::scoped_nsobject<CRWJSWindowIdManager> manager_;
// WebClient required for getting early page script, which must be injected
// before CRWJSWindowIdManager.
web::WebClient web_client_;
web::ScopedTestingWebClient web_client_;
};

// Tests that reinjection of window ID JS results in a different window ID.
Expand Down
11 changes: 8 additions & 3 deletions ios/web/web_state/ui/crw_wk_script_message_router_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "base/mac/scoped_block.h"
#include "base/mac/scoped_nsobject.h"
#include "ios/web/public/test/scoped_testing_web_client.h"
#include "ios/web/public/test/test_browser_state.h"
#import "ios/web/public/test/test_web_client.h"
#include "ios/web/public/test/web_test_util.h"
Expand All @@ -25,10 +26,14 @@ id GetScriptMessageMock(WKWebView* web_view, NSString* name) {

// Test fixture for CRWWKScriptMessageRouter.
class CRWWKScriptMessageRouterTest : public web::WebTest {
public:
CRWWKScriptMessageRouterTest()
: web_client_(make_scoped_ptr(new web::WebClient)) {}

protected:
void SetUp() override {
web::WebTest::SetUp();
CR_TEST_REQUIRES_WK_WEB_VIEW();
web::SetWebClient(&web_client_);
// Mock WKUserContentController object.
controller_mock_.reset(
[[OCMockObject mockForClass:[WKUserContentController class]] retain]);
Expand All @@ -55,7 +60,7 @@ void SetUp() override {
}
void TearDown() override {
EXPECT_OCMOCK_VERIFY(controller_mock_);
web::SetWebClient(nullptr);
web::WebTest::TearDown();
}

// WKUserContentController mock used to create testable router.
Expand All @@ -78,7 +83,7 @@ void TearDown() override {

private:
// WebClient and BrowserState for testing.
web::TestWebClient web_client_;
web::ScopedTestingWebClient web_client_;
web::TestBrowserState browser_state_;
};

Expand Down
Loading

0 comments on commit 18f952a

Please sign in to comment.