summaryrefslogtreecommitdiffstats
path: root/ppapi
diff options
context:
space:
mode:
authordmichael@chromium.org <dmichael@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-07-17 20:15:21 +0000
committerdmichael@chromium.org <dmichael@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-07-17 20:15:21 +0000
commite7425db91f416ee6913d8c59388d243fcbff6739 (patch)
tree49ab8bf1416160bd88e637fcec1ca556b189152e /ppapi
parent620c7974068d3b70b07917f883c638c5af388a9c (diff)
downloadchromium_src-e7425db91f416ee6913d8c59388d243fcbff6739.zip
chromium_src-e7425db91f416ee6913d8c59388d243fcbff6739.tar.gz
chromium_src-e7425db91f416ee6913d8c59388d243fcbff6739.tar.bz2
PPAPI: Test cleanup, make postconditions work
This CL does a few things for our tests: - Remove cookie-related junk that is no longer valid. We use domAutomationController to signal test completion/progress/failure now, not cookies. - Make CheckPostConditions work again by using domAutomationController instead of the cookies, which were ignored. - Change the way we decide whether to remove the plugin after the test is run. - Update TestInstanceDeprecated to better reflect actual shutdown behavior. R=bbudge@chromium.org, teravest@chromium.org Review URL: https://codereview.chromium.org/15976003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@212121 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'ppapi')
-rw-r--r--ppapi/tests/test_case.html65
-rw-r--r--ppapi/tests/test_instance_deprecated.cc55
-rw-r--r--ppapi/tests/test_instance_deprecated.h1
-rw-r--r--ppapi/tests/testing_instance.cc18
4 files changed, 83 insertions, 56 deletions
diff --git a/ppapi/tests/test_case.html b/ppapi/tests/test_case.html
index 4b67a0f..c963b13 100644
--- a/ppapi/tests/test_case.html
+++ b/ppapi/tests/test_case.html
@@ -9,11 +9,16 @@ function AdjustHeight(frameWin) {
frameWin.frameElement.style.height = height;
}
-function DidExecuteTests() {
+// Called when the tests are completed. |result| should be "PASS" if the test(s)
+// passed, or information about the failure if the test(s) did not pass.
+function DidExecuteTests(result) {
var plugin = document.getElementById("plugin");
- plugin.parentNode.removeChild(plugin);
- plugin = undefined;
- CheckPostConditions();
+ if (plugin.removePlugin) {
+ plugin.parentNode.removeChild(plugin);
+ plugin = undefined;
+ }
+ if (CheckPostConditions())
+ sendAutomationMessage(result);
if (window == top)
return;
@@ -89,12 +94,13 @@ function ExtractSearchParameter(name) {
//
// Otherwise, returns an array containing 2 items. The 0th element is the
// message_type, one of:
-// - ClearContents
+// - AddPostCondition
+// - ClearConsole
// - DidExecuteTests
-// - LogHTML
-// - SetCookie
// - EvalScript
-// - AddPostCondition
+// - LogHTML
+// - RemovePluginWhenFinished
+// - ReportProgress
// The second item is the verbatim message_contents.
function ParseTestingMessage(message_data) {
if (typeof(message_data) != "string")
@@ -120,6 +126,10 @@ function LogHTML(html) {
window.document.getElementById("console").innerHTML += html;
}
+function RemovePluginWhenFinished() {
+ window.document.getElementById("plugin").removePlugin = true;
+}
+
function sendAutomationMessage(msg) {
if (window.domAutomationController) {
window.domAutomationController.setAutomationId(0);
@@ -136,10 +146,6 @@ function InternalError(msg) {
sendAutomationMessage(msg);
}
-function SetCookie(key_value_string) {
- window.document.cookie = key_value_string + "; path=/";
-}
-
function EvalScript(script) {
try {
eval(script);
@@ -158,33 +164,26 @@ function AddPostCondition(script) {
// doesn't count this as a pass.
function ConditionFailed(error) {
error_string = "Post condition check failed: " + error;
- var i = 0;
- // If the plugin thinks the test passed but a post-condition failed, we want
- // to clear the PASS cookie so that ui_tests doesn't count it is a passed
- // test.
- if (window.document.cookie.indexOf("PASS") != -1) {
- while (window.document.cookie.indexOf("PPAPI_PROGRESS_" + i) != -1) {
- window.document.cookie = "PPAPI_PROGRESS_" + i + "=; max-age=0";
- ++i;
- }
- window.document.cookie = "PPAPI_PROGRESS_0=" + error_string
- }
- LogHTML("<p>" + error_string);
+ InternalError(error_string);
}
// Iterate through the post conditions defined in |conditions| and check that
// they all pass.
function CheckPostConditions() {
+ var success = true;
for (var i = 0; i < conditions.length; ++i) {
var script = conditions[i];
try {
if (!eval(script)) {
ConditionFailed("\"" + script + "\"");
+ success = false;
}
} catch (ex) {
ConditionFailed("\"" + script + "\"" + " failed with exception: " +
"\"" + ex.toString() + "\"");
+ success = false;
}
}
+ return success;
}
function IsTestingMessage(message_data) {
@@ -196,18 +195,20 @@ function handleTestingMessage(message_event) {
if (type_contents_tuple) {
var type = type_contents_tuple[0];
var contents = type_contents_tuple[1];
- if (type === "ClearConsole")
+ if (type === "AddPostCondition")
+ AddPostCondition(contents);
+ else if (type === "ClearConsole")
ClearConsole();
else if (type === "DidExecuteTests")
- DidExecuteTests();
- else if (type === "LogHTML")
- LogHTML(contents);
- else if (type === "SetCookie")
- SetCookie(contents);
+ DidExecuteTests(contents);
else if (type === "EvalScript")
EvalScript(contents);
- else if (type == "AddPostCondition")
- AddPostCondition(contents);
+ else if (type === "LogHTML")
+ LogHTML(contents);
+ else if (type === "RemovePluginWhenFinished")
+ RemovePluginWhenFinished();
+ else if (type === "ReportProgress")
+ sendAutomationMessage(contents);
}
}
diff --git a/ppapi/tests/test_instance_deprecated.cc b/ppapi/tests/test_instance_deprecated.cc
index 72e1fad..48e8dbb 100644
--- a/ppapi/tests/test_instance_deprecated.cc
+++ b/ppapi/tests/test_instance_deprecated.cc
@@ -5,6 +5,7 @@
#include "ppapi/tests/test_instance_deprecated.h"
#include <assert.h>
+#include <iostream>
#include "ppapi/c/ppb_var.h"
#include "ppapi/cpp/module.h"
@@ -20,7 +21,7 @@ static const char kReturnValueFunction[] = "ReturnValue";
// ScriptableObject used by instance.
class InstanceSO : public pp::deprecated::ScriptableObject {
public:
- InstanceSO(TestInstance* i);
+ explicit InstanceSO(TestInstance* i);
virtual ~InstanceSO();
// pp::deprecated::ScriptableObject overrides.
@@ -31,27 +32,50 @@ class InstanceSO : public pp::deprecated::ScriptableObject {
private:
TestInstance* test_instance_;
+ // For out-of-process, the InstanceSO might be deleted after the instance was
+ // already destroyed, so we can't rely on test_instance_->testing_interface()
+ // being valid. Therefore we store our own.
+ const PPB_Testing_Dev* testing_interface_;
};
-InstanceSO::InstanceSO(TestInstance* i) : test_instance_(i) {
+InstanceSO::InstanceSO(TestInstance* i)
+ : test_instance_(i),
+ testing_interface_(i->testing_interface()) {
// Set up a post-condition for the test so that we can ensure our destructor
- // is called. This only works in-process right now. Rather than disable the
- // whole test, we only do this check when running in-process.
- // TODO(dmichael): Figure out if we want this to work out-of-process, and if
- // so, fix it. Note that it might just be failing because the
- // ReleaseObject and Deallocate messages are asynchronous.
- if (i->testing_interface() &&
- i->testing_interface()->IsOutOfProcess() == PP_FALSE) {
+ // is called. This only works reliably in-process. Out-of-process, it only
+ // can work when the renderer stays alive a short while after the plugin
+ // instance is destroyed. If the renderer is being shut down, too much happens
+ // asynchronously for the out-of-process case to work reliably. In
+ // particular:
+ // - The Var ReleaseObject message is asynchronous.
+ // - The PPB_Var_Deprecated host-side proxy posts a task to actually release
+ // the object when the ReleaseObject message is received.
+ // - The PPP_Class Deallocate message is asynchronous.
+ // At time of writing this comment, if you modify the code so that the above
+ // happens synchronously, and you remove the restriction that the plugin can't
+ // be unblocked by a sync message, then this check actually passes reliably
+ // for out-of-process. But we don't want to make any of those changes, so we
+ // just skip the check.
+ if (testing_interface_->IsOutOfProcess() == PP_FALSE) {
i->instance()->AddPostCondition(
- "window.document.getElementById('container').instance_object_destroyed"
- );
+ "window.document.getElementById('container').instance_object_destroyed"
+ );
}
}
InstanceSO::~InstanceSO() {
- pp::Var exception;
- pp::Var ret = test_instance_->instance()->ExecuteScript(
- "document.getElementById('container').instance_object_destroyed=true;");
+ if (testing_interface_->IsOutOfProcess() == PP_FALSE) {
+ // TODO(dmichael): It would probably be best to make in-process consistent
+ // with out-of-process. That would mean that the instance
+ // would already be destroyed at this point.
+ pp::Var exception;
+ pp::Var ret = test_instance_->instance()->ExecuteScript(
+ "document.getElementById('container').instance_object_destroyed=true;");
+ } else {
+ // Out-of-process, this destructor might not actually get invoked. See the
+ // comment in InstanceSO's constructor for an explanation. Also, instance()
+ // has already been destroyed :-(. So we can't really do anything here.
+ }
}
bool InstanceSO::HasMethod(const pp::Var& name, pp::Var* exception) {
@@ -102,6 +126,9 @@ bool TestInstance::Init() {
return true;
}
+TestInstance::~TestInstance() {
+}
+
void TestInstance::RunTests(const std::string& filter) {
RUN_TEST(ExecuteScript, filter);
RUN_TEST(RecursiveObjects, filter);
diff --git a/ppapi/tests/test_instance_deprecated.h b/ppapi/tests/test_instance_deprecated.h
index 6ec9b46..eba2909 100644
--- a/ppapi/tests/test_instance_deprecated.h
+++ b/ppapi/tests/test_instance_deprecated.h
@@ -13,6 +13,7 @@
class TestInstance : public TestCase {
public:
TestInstance(TestingInstance* instance);
+ virtual ~TestInstance();
// TestCase implementation.
virtual bool Init();
diff --git a/ppapi/tests/testing_instance.cc b/ppapi/tests/testing_instance.cc
index 465bbb1..ea31322 100644
--- a/ppapi/tests/testing_instance.cc
+++ b/ppapi/tests/testing_instance.cc
@@ -217,12 +217,14 @@ void TestingInstance::ExecuteTests(int32_t unused) {
}
}
- // Declare we're done by setting a cookie to either "PASS" or the errors.
- ReportProgress(errors_.empty() ? "PASS" : errors_);
if (remove_plugin_)
- SendTestCommand("DidExecuteTests");
- // Note, DidExecuteTests unloads the plugin. We can't really do anthing after
- // this point.
+ SendTestCommand("RemovePluginWhenFinished");
+ std::string result(errors_);
+ if (result.empty())
+ result = "PASS";
+ SendTestCommand("DidExecuteTests", result);
+ // Note, DidExecuteTests may unload the plugin. We can't really do anything
+ // after this point.
}
TestCase* TestingInstance::CaseForTestName(const std::string& name) {
@@ -288,11 +290,7 @@ void TestingInstance::LogHTML(const std::string& html) {
}
void TestingInstance::ReportProgress(const std::string& progress_value) {
- // Use streams since nacl doesn't compile base yet (for StringPrintf).
- std::ostringstream script;
- script << "window.domAutomationController.setAutomationId(0);" <<
- "window.domAutomationController.send(\"" << progress_value << "\")";
- EvalScript(script.str());
+ SendTestCommand("ReportProgress", progress_value);
}
void TestingInstance::AddPostCondition(const std::string& script) {