Skip to content

Commit

Permalink
Add support for deferring app initialization when testing.
Browse files Browse the repository at this point in the history
This will allow certain app features such as authentication and loading of the
host list to be replaced with mocks before they are used; in normal operation,
these features are run automatically when the app is loaded, giving browser
tests no opportunity to intervene.

This is achieved by using the "source" URL parameter. Production code is
unaffected by virtue of the fact that "source" is not passed by default, and
even if a user is running with a command-line override, it will never be set
to "test". As protection against the possibility that the previous sentence
is false, when started in this test mode, initialization proceeds via a
button click, making it user-accessible; this CL also adds a test that this
button works.

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

Cr-Commit-Position: refs/heads/master@{#308513}
  • Loading branch information
jamiewalch authored and Commit bot committed Dec 16, 2014
1 parent 622ee17 commit 0932e48
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 22 deletions.
2 changes: 1 addition & 1 deletion chrome/test/remoting/auth_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ IN_PROC_BROWSER_TEST_F(RemoteDesktopBrowserTest, MANUAL_Auth) {

Install();

LaunchChromotingApp();
LaunchChromotingApp(false);

// Authorize, Authenticate, and Approve.
Auth();
Expand Down
13 changes: 12 additions & 1 deletion chrome/test/remoting/launch_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,18 @@ IN_PROC_BROWSER_TEST_F(RemoteDesktopBrowserTest, MANUAL_Launch) {

Install();

LaunchChromotingApp();
LaunchChromotingApp(false);

Cleanup();
}

IN_PROC_BROWSER_TEST_F(RemoteDesktopBrowserTest, MANUAL_LaunchDeferredStart) {
VerifyInternetAccess();

Install();

LaunchChromotingApp(true);
StartChromotingApp();

Cleanup();
}
Expand Down
4 changes: 2 additions & 2 deletions chrome/test/remoting/me2me_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ IN_PROC_BROWSER_TEST_F(Me2MeBrowserTest,
MANUAL_Me2Me_Connect_Remote_Host) {
VerifyInternetAccess();
Install();
LaunchChromotingApp();
LaunchChromotingApp(false);

// Authorize, Authenticate, and Approve.
Auth();
Expand Down Expand Up @@ -110,7 +110,7 @@ void Me2MeBrowserTest::ConnectPinlessAndCleanupPairings(bool cleanup_all) {
// TODO(jamiewalch): This reload is only needed because there's a bug in the
// web-app whereby it doesn't refresh its pairing state correctly.
// http://crbug.com/311290
LaunchChromotingApp();
LaunchChromotingApp(false);
ASSERT_TRUE(HtmlElementVisible("paired-client-manager-message"));

// Second connection: verify that no PIN is requested.
Expand Down
13 changes: 11 additions & 2 deletions chrome/test/remoting/page_load_notification_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,24 @@ PageLoadNotificationObserver::PageLoadNotificationObserver(const GURL& target)
content::NOTIFICATION_LOAD_STOP,
base::Bind(&PageLoadNotificationObserver::IsTargetLoaded,
base::Unretained(this))),
target_(target) {
target_(target),
ignore_url_parameters_(false) {
}

PageLoadNotificationObserver::~PageLoadNotificationObserver() {}

bool PageLoadNotificationObserver::IsTargetLoaded() {
content::NavigationController* controller =
content::Source<content::NavigationController>(source()).ptr();
return controller->GetWebContents()->GetURL() == target_;
GURL current_url = controller->GetWebContents()->GetURL();
if (ignore_url_parameters_) {
GURL::Replacements strip_query;
strip_query.ClearQuery();
return current_url.ReplaceComponents(strip_query) ==
target_.ReplaceComponents(strip_query);
} else {
return current_url == target_;
}
}

} // namespace remoting
7 changes: 7 additions & 0 deletions chrome/test/remoting/page_load_notification_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,16 @@ class PageLoadNotificationObserver

~PageLoadNotificationObserver() override;

void set_ignore_url_parameters(bool ignore_url_parameters) {
ignore_url_parameters_ = ignore_url_parameters;
}

private:
bool IsTargetLoaded();

GURL target_;
bool ignore_url_parameters_;

DISALLOW_COPY_AND_ASSIGN(PageLoadNotificationObserver);
};

Expand Down
16 changes: 14 additions & 2 deletions chrome/test/remoting/remote_desktop_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,22 @@ void RemoteDesktopBrowserTest::VerifyChromotingLoaded(bool expected) {
ASSERT_EQ(installed, expected);
}

void RemoteDesktopBrowserTest::LaunchChromotingApp() {
void RemoteDesktopBrowserTest::LaunchChromotingApp(bool defer_start) {
ASSERT_TRUE(extension_);

GURL chromoting_main = Chromoting_Main_URL();
// We cannot simply wait for any page load because the first page
// loaded could be the generated background page. We need to wait
// till the chromoting main page is loaded.
PageLoadNotificationObserver observer(chromoting_main);
observer.set_ignore_url_parameters(true);

// If the app should be started in deferred mode, ensure that a "source" URL
// parameter; if not, ensure that no such parameter is present. The value of
// the parameter is determined by the AppLaunchParams ("test", in this case).
extensions::FeatureSwitch::ScopedOverride override_trace_app_source(
extensions::FeatureSwitch::trace_app_source(),
defer_start);

OpenApplication(AppLaunchParams(browser()->profile(), extension_,
is_platform_app()
Expand Down Expand Up @@ -210,6 +218,10 @@ void RemoteDesktopBrowserTest::LaunchChromotingApp() {
EXPECT_EQ(Chromoting_Main_URL(), GetCurrentURL());
}

void RemoteDesktopBrowserTest::StartChromotingApp() {
ClickOnControl("browser-test-continue-init");
};

void RemoteDesktopBrowserTest::Authorize() {
// The chromoting extension should be installed.
ASSERT_TRUE(extension_);
Expand Down Expand Up @@ -465,7 +477,7 @@ void RemoteDesktopBrowserTest::Cleanup() {
void RemoteDesktopBrowserTest::SetUpTestForMe2Me() {
VerifyInternetAccess();
Install();
LaunchChromotingApp();
LaunchChromotingApp(false);
Auth();
LoadScript(app_web_content(), FILE_PATH_LITERAL("browser_test.js"));
ExpandMe2Me();
Expand Down
29 changes: 17 additions & 12 deletions chrome/test/remoting/remote_desktop_browsertest.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,16 @@ class RemoteDesktopBrowserTest : public extensions::PlatformAppBrowserTest {
// Test whether the chromoting extension is installed.
void VerifyChromotingLoaded(bool expected);

// Launch the chromoting app.
void LaunchChromotingApp();
// Launch the Chromoting app. If |defer_start| is true, an additional URL
// parameter is passed to the application, causing it to defer start-up
// until StartChromotingApp is invoked. Test code can execute arbitrary
// JavaScript in the context of the app between these two calls, for example
// to set up appropriate mocks.
void LaunchChromotingApp(bool defer_start);

// If the Chromoting app was launched in deferred mode, tell it to continue
// its regular start-up sequence.
void StartChromotingApp();

// Authorize: grant extended access permission to the user's computer.
void Authorize();
Expand Down Expand Up @@ -223,19 +231,16 @@ class RemoteDesktopBrowserTest : public extensions::PlatformAppBrowserTest {

// Helper to construct the starting URL of the installed chromoting webapp.
GURL Chromoting_Main_URL() {
if (is_platform_app())
// The v2 remoting app recently (M38 at the latest) started adding a
// query-string parameter to the main extension page. So we'll create a
// different expected URL for it.
return GURL("chrome-extension://" + ChromotingID() +
"/main.html?isKioskSession=false");
else
return GURL("chrome-extension://" + ChromotingID() + "/main.html");
return GURL("chrome-extension://" + ChromotingID() + "/main.html");
}

// Helper to retrieve the current URL in the active WebContents.
// Helper to retrieve the current URL in the active WebContents. This function
// strips all query parameters from the URL.
GURL GetCurrentURL() {
return active_web_contents()->GetURL();
GURL current_url = active_web_contents()->GetURL();
GURL::Replacements strip_query;
strip_query.ClearQuery();
return current_url.ReplaceComponents(strip_query);
}

// Helpers to execute JavaScript code on a web page.
Expand Down
9 changes: 9 additions & 0 deletions remoting/webapp/crd/html/template_main.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@
hidden, but are shown appropriately when the mode changes. -->
<section id="loading-mode" data-ui-mode="">
<em>Loading&hellip;</em>
<div id="browser-test-deferred-init" hidden>
<p>
Running in test mode. If you are seeing this message in any other
context, please <a href="http://crbug.com" target="_blank">file a
bug</a>.
</p>
<p>
<button id="browser-test-continue-init">Continue</button>
</p>
</section>

<div id="daemon-plugin-container"></div>
Expand Down
23 changes: 21 additions & 2 deletions remoting/webapp/crd/js/crd_main.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,31 @@ remoting.updateLocalHostState = function() {
};

/**
* Entry point ('load' handler) for Remote Desktop (CRD) webapp.
* Entry point for test code. In order to make test and production
* code as similar as possible, the same entry point is used for
* production code, but since production code should never have
* 'source' set to 'test', it continue with initialization
* immediately. As a fail-safe, the mechanism by which initialization
* completes when under test is controlled by a simple UI, making it
* possible to use the app even if the previous assumption fails to
* hold in some corner cases.
*/
remoting.initDesktopRemotingForTesting = function() {
var urlParams = getUrlParameters_();
if (urlParams['source'] === 'test') {
document.getElementById('browser-test-continue-init').addEventListener(
'click', remoting.initDesktopRemoting, false);
document.getElementById('browser-test-deferred-init').hidden = false;
} else {
remoting.initDesktopRemoting();
}
}


remoting.initDesktopRemoting = function() {
remoting.app = new remoting.Application();
var desktop_remoting = new remoting.DesktopRemoting(remoting.app);
remoting.app.init();
};

window.addEventListener('load', remoting.initDesktopRemoting, false);
window.addEventListener('load', remoting.initDesktopRemotingForTesting, false);

0 comments on commit 0932e48

Please sign in to comment.