diff options
| -rw-r--r-- | chrome/test/ui/ppapi_uitest.cc | 57 | ||||
| -rw-r--r-- | ppapi/tests/test_case.cc | 43 | ||||
| -rw-r--r-- | ppapi/tests/test_case.h | 12 | ||||
| -rw-r--r-- | ppapi/tests/test_case.html | 53 | ||||
| -rw-r--r-- | ppapi/tests/test_instance_deprecated.cc | 102 | ||||
| -rw-r--r-- | ppapi/tests/test_instance_deprecated.h | 9 | ||||
| -rw-r--r-- | ppapi/tests/testing_instance.cc | 38 | ||||
| -rw-r--r-- | ppapi/tests/testing_instance.h | 11 | ||||
| -rw-r--r-- | webkit/plugins/ppapi/host_var_tracker.cc | 31 | ||||
| -rw-r--r-- | webkit/plugins/ppapi/host_var_tracker.h | 10 | ||||
| -rw-r--r-- | webkit/plugins/ppapi/message_channel.cc | 5 | ||||
| -rw-r--r-- | webkit/plugins/ppapi/npobject_var.h | 6 | ||||
| -rw-r--r-- | webkit/plugins/ppapi/ppapi_plugin_instance.cc | 5 | ||||
| -rw-r--r-- | webkit/plugins/ppapi/ppapi_webplugin_impl.cc | 19 | ||||
| -rw-r--r-- | webkit/plugins/ppapi/ppapi_webplugin_impl.h | 2 |
15 files changed, 65 insertions, 338 deletions
diff --git a/chrome/test/ui/ppapi_uitest.cc b/chrome/test/ui/ppapi_uitest.cc index 939afad..03c40ae 100644 --- a/chrome/test/ui/ppapi_uitest.cc +++ b/chrome/test/ui/ppapi_uitest.cc @@ -70,42 +70,12 @@ class PPAPITestBase : public UITest { void RunTest(const std::string& test_case) { scoped_refptr<TabProxy> tab = GetActiveTab(); - ASSERT_TRUE(tab.get()); - GURL url = GetTestFileUrl(test_case); - EXPECT_TRUE(tab->NavigateToURLBlockUntilNavigationsComplete(url, 1)); - RunTestURL(tab, url); - } - - // Run the test and reload. This can test for clean shutdown, including leaked - // instance object vars. - void RunTestAndReload(const std::string& test_case) { - scoped_refptr<TabProxy> tab = GetActiveTab(); - ASSERT_TRUE(tab.get()); + EXPECT_TRUE(tab.get()); + if (!tab.get()) + return; GURL url = GetTestFileUrl(test_case); - // Run the test the first time. EXPECT_TRUE(tab->NavigateToURLBlockUntilNavigationsComplete(url, 1)); RunTestURL(tab, url); - // Now we'll clean up the cookies to prepare for reloading and running the - // test again. Look for the first cookie that isn't equal to "...". It - // will either be "PASS" or errors from the test. - int progress_number = 0; - std::string cookie_name, progress("..."); - while (progress == "...") { - cookie_name = StringPrintf("PPAPI_PROGRESS_%d", progress_number); - progress = WaitUntilCookieNonEmpty(tab.get(), url, cookie_name.c_str(), - TestTimeouts::action_timeout_ms()); - ++progress_number; - } - // If the first run passed, then delete the "PASS" cookie and reload. If we - // left the "PASS" cookie alone, RunTestURL would find it and assume we - // passed the test on the reload regardless of the actual result. If the - // first run failed, we let those errors take precedent and don't bother - // reloading and running the test again. - if (progress == "PASS") { - EXPECT_TRUE(tab->DeleteCookie(url, cookie_name)); - EXPECT_TRUE(tab->Reload()); - RunTestURL(tab, url); - } } void RunTestViaHTTP(const std::string& test_case) { @@ -397,23 +367,10 @@ TEST_PPAPI_OUT_OF_PROCESS(MAYBE_InputEvent) // TODO(bbudge) Enable when input events are proxied correctly for NaCl. TEST_PPAPI_NACL_VIA_HTTP(DISABLED_InputEvent) -TEST_PPAPI_IN_PROCESS(Instance_ExecuteScript); -TEST_PPAPI_OUT_OF_PROCESS(Instance_ExecuteScript) -// ExecuteScript isn't supported by NaCl. - -// We run and reload the RecursiveObjects test to ensure that the InstanceObject -// (and others) are properly cleaned up after the first run. -TEST_F(PPAPITest, Instance_RecursiveObjects) { - RunTestAndReload("Instance_RecursiveObjects"); -} -// TODO(dmichael): Make it work out-process (or at least see whether we care). -TEST_F(OutOfProcessPPAPITest, DISABLED_Instance_RecursiveObjects) { - RunTestAndReload("Instance_RecursiveObjects"); -} -TEST_PPAPI_IN_PROCESS(Instance_TestLeakedObjectDestructors); -TEST_PPAPI_OUT_OF_PROCESS(Instance_TestLeakedObjectDestructors); -// ScriptableObjects aren't supported in NaCl, so Instance_RecursiveObjects and -// Instance_TestLeakedObjectDestructors don't make sense for NaCl. +TEST_PPAPI_IN_PROCESS(Instance) +TEST_PPAPI_OUT_OF_PROCESS(Instance) +// The Instance test is actually InstanceDeprecated which isn't supported +// by NaCl. TEST_PPAPI_IN_PROCESS(Graphics2D) TEST_PPAPI_OUT_OF_PROCESS(Graphics2D) diff --git a/ppapi/tests/test_case.cc b/ppapi/tests/test_case.cc index 0100ded..90e7d72 100644 --- a/ppapi/tests/test_case.cc +++ b/ppapi/tests/test_case.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -47,13 +47,8 @@ std::string TestCase::MakeFailureMessage(const char* file, pp::VarPrivate TestCase::GetTestObject() { if (test_object_.is_undefined()) { pp::deprecated::ScriptableObject* so = CreateTestObject(); - if (so) { + if (so) test_object_ = pp::VarPrivate(instance_, so); // Takes ownership. - // CheckResourcesAndVars runs and looks for leaks before we've actually - // completely shut down. Ignore the instance object, since it's not a real - // leak. - IgnoreLeakedVar(test_object_.pp_var().value.as_id); - } } return test_object_; } @@ -83,10 +78,6 @@ bool TestCase::HandleInputEvent(const pp::InputEvent& event) { return false; } -void TestCase::IgnoreLeakedVar(int64_t id) { - ignored_leaked_vars_.insert(id); -} - #if !(defined __native_client__) pp::deprecated::ScriptableObject* TestCase::CreateTestObject() { return NULL; @@ -120,27 +111,31 @@ std::string TestCase::CheckResourcesAndVars() { errors += output.str(); } */ - const int kVarsToPrint = 100; + const uint32_t kVarsToPrint = 10; PP_Var vars[kVarsToPrint]; - int found_vars = testing_interface_->GetLiveVars(vars, kVarsToPrint); - // This will undercount if we are told to ignore a Var which is then *not* - // leaked. Worst case, we just won't print the little "Test leaked" message, - // but we'll still print any non-ignored leaked vars we found. - int leaked_vars = - found_vars - static_cast<int>(ignored_leaked_vars_.size()); - if (leaked_vars > 0) { + uint32_t leaked_vars = testing_interface_->GetLiveVars(vars, kVarsToPrint); + uint32_t tracked_vars = leaked_vars; +#if !(defined __native_client__) + // Don't count test_object_ as a leak. + if (test_object_.pp_var().type > PP_VARTYPE_DOUBLE) + --leaked_vars; +#endif + if (leaked_vars) { std::ostringstream output; output << "Test leaked " << leaked_vars << " vars (printing at most " << kVarsToPrint <<"):<p>"; errors += output.str(); - } - for (int i = 0; i < std::min(found_vars, kVarsToPrint); ++i) { - pp::Var leaked_var(pp::PASS_REF, vars[i]); - if (ignored_leaked_vars_.count(leaked_var.pp_var().value.as_id) == 0) + for (uint32_t i = 0; i < std::min(tracked_vars, kVarsToPrint); ++i) { + pp::Var leaked_var(pp::PASS_REF, vars[i]); +#if (defined __native_client__) errors += leaked_var.DebugString() + "<p>"; +#else + if (!(leaked_var == test_object_)) + errors += leaked_var.DebugString() + "<p>"; +#endif + } } } return errors; } - diff --git a/ppapi/tests/test_case.h b/ppapi/tests/test_case.h index ee22f17..c04ed83 100644 --- a/ppapi/tests/test_case.h +++ b/ppapi/tests/test_case.h @@ -7,7 +7,6 @@ #include <cmath> #include <limits> -#include <set> #include <string> #include "ppapi/c/pp_resource.h" @@ -71,16 +70,10 @@ class TestCase { // that want to handle view changes should override this method. virtual bool HandleInputEvent(const pp::InputEvent& event); - void IgnoreLeakedVar(int64_t id); - - TestingInstance* instance() { return instance_; } - - const PPB_Testing_Dev* testing_interface() { return testing_interface_; } - protected: #if !(defined __native_client__) // Overridden by each test to supply a ScriptableObject corresponding to the - // test. There can only be one object created for all tests in a given class, + // test. There can only be one object created for all test in a given class // so be sure your object is designed to be re-used. // // This object should be created on the heap. Ownership will be passed to the @@ -115,9 +108,6 @@ class TestCase { bool force_async_; private: - // Var ids that should be ignored when checking for leaks on shutdown. - std::set<int64_t> ignored_leaked_vars_; - #if !(defined __native_client__) // Holds the test object, if any was retrieved from CreateTestObject. pp::VarPrivate test_object_; diff --git a/ppapi/tests/test_case.html b/ppapi/tests/test_case.html index 92aebdb..f5c046e 100644 --- a/ppapi/tests/test_case.html +++ b/ppapi/tests/test_case.html @@ -10,11 +10,6 @@ function AdjustHeight(frameWin) { } function DidExecuteTests() { - var plugin = document.getElementById("plugin"); - plugin.parentNode.removeChild(plugin); - plugin = undefined; - CheckPostConditions(); - if (window == top) return; @@ -84,10 +79,9 @@ function ExtractSearchParameter(name) { // - LogHTML // - SetCookie // - EvalScript -// - AddPostCondition // The second item is the verbatim message_contents. function ParseTestingMessage(message_data) { - if (typeof(message_data) != "string") + if (typeof(message_data)!='string') return undefined; var testing_message_prefix = "TESTING_MESSAGE"; var delim_str = ":"; @@ -121,46 +115,6 @@ function EvalScript(script) { } } -var conditions = []; -// Add a "PostCondition". These are bits of script that are run after the plugin -// is destroyed. If they evaluate to false or throw an exception, it's -// considered a failure. -function AddPostCondition(script) { - conditions.push(script); -} -// Update the HTML to show the failure and update cookies so that ui_tests -// 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); -} -// Iterate through the post conditions defined in |conditions| and check that -// they all pass. -function CheckPostConditions() { - for (var i = 0; i < conditions.length; ++i) { - var script = conditions[i]; - try { - if (!eval(script)) { - ConditionFailed("\"" + script + "\""); - } - } catch (ex) { - ConditionFailed("\"" + script + "\"" + " failed with exception: " + - "\"" + ex.toString() + "\""); - } - } -} - function IsTestingMessage(message_data) { return (ParseTestingMessage(message_data) != undefined); } @@ -180,8 +134,6 @@ function handleTestingMessage(message_event) { SetCookie(contents); else if (type === "EvalScript") EvalScript(contents); - else if (type == "AddPostCondition") - AddPostCondition(contents); } } @@ -232,9 +184,6 @@ onload = function() { // particular, we replace document.createEvent, MessageEvent.initMessageEvent, // and the MessageEvent constructor. Previous versions of the PostMessage // implementation made use of these and would fail (http://crbug.com/82604). -// TODO(dmichael): These should clear the PASS cookie so that ui_tests sees -// these as a failure (see ConditionFailed for how to do this). -// crbug.com/109775 document.createEvent = function() { LogHTML("<p>Bad document.createEvent called!"); } diff --git a/ppapi/tests/test_instance_deprecated.cc b/ppapi/tests/test_instance_deprecated.cc index 72e1fad..e7ac48b 100644 --- a/ppapi/tests/test_instance_deprecated.cc +++ b/ppapi/tests/test_instance_deprecated.cc @@ -1,12 +1,9 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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 "ppapi/tests/test_instance_deprecated.h" -#include <assert.h> - -#include "ppapi/c/ppb_var.h" #include "ppapi/cpp/module.h" #include "ppapi/cpp/dev/scriptable_object_deprecated.h" #include "ppapi/tests/testing_instance.h" @@ -20,8 +17,7 @@ static const char kReturnValueFunction[] = "ReturnValue"; // ScriptableObject used by instance. class InstanceSO : public pp::deprecated::ScriptableObject { public: - InstanceSO(TestInstance* i); - virtual ~InstanceSO(); + InstanceSO(TestInstance* i) : test_instance_(i) {} // pp::deprecated::ScriptableObject overrides. bool HasMethod(const pp::Var& name, pp::Var* exception); @@ -33,27 +29,6 @@ class InstanceSO : public pp::deprecated::ScriptableObject { TestInstance* test_instance_; }; -InstanceSO::InstanceSO(TestInstance* i) : test_instance_(i) { - // 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) { - i->instance()->AddPostCondition( - "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;"); -} - bool InstanceSO::HasMethod(const pp::Var& name, pp::Var* exception) { if (!name.is_string()) return false; @@ -104,15 +79,6 @@ bool TestInstance::Init() { void TestInstance::RunTests(const std::string& filter) { RUN_TEST(ExecuteScript, filter); - RUN_TEST(RecursiveObjects, filter); - RUN_TEST(LeakedObjectDestructors, filter); -} - -void TestInstance::LeakReferenceAndIgnore(const pp::Var& leaked) { - static const PPB_Var* var_interface = static_cast<const PPB_Var*>( - pp::Module::Get()->GetBrowserInterface(PPB_VAR_INTERFACE)); - var_interface->AddRef(leaked.pp_var()); - IgnoreLeakedVar(leaked.pp_var().value.as_id); } pp::deprecated::ScriptableObject* TestInstance::CreateTestObject() { @@ -154,67 +120,3 @@ std::string TestInstance::TestExecuteScript() { PASS(); } - -// A scriptable object that contains other scriptable objects recursively. This -// is used to help verify that our scriptable object clean-up code works -// properly. -class ObjectWithChildren : public pp::deprecated::ScriptableObject { - public: - ObjectWithChildren(TestInstance* i, int num_descendents) { - if (num_descendents > 0) { - child_ = pp::VarPrivate(i->instance(), - new ObjectWithChildren(i, num_descendents - 1)); - } - } - struct IgnoreLeaks {}; - ObjectWithChildren(TestInstance* i, int num_descendents, IgnoreLeaks) { - if (num_descendents > 0) { - child_ = pp::VarPrivate(i->instance(), - new ObjectWithChildren(i, num_descendents - 1, - IgnoreLeaks())); - i->IgnoreLeakedVar(child_.pp_var().value.as_id); - } - } - private: - pp::VarPrivate child_; -}; - -std::string TestInstance::TestRecursiveObjects() { - // These should be deleted when we exit scope, so should not leak. - pp::VarPrivate not_leaked(instance(), new ObjectWithChildren(this, 50)); - - // Leak some, but tell TestCase to ignore the leaks. This test is run and then - // reloaded (see ppapi_uitest.cc). If these aren't cleaned up when the first - // run is torn down, they will show up as leaks in the second run. - // NOTE: The ScriptableObjects are actually leaked, but they should be removed - // from the tracker. See below for a test that verifies that the - // destructor is not run. - pp::VarPrivate leaked( - instance(), - new ObjectWithChildren(this, 50, ObjectWithChildren::IgnoreLeaks())); - // Now leak a reference to the root object. This should force the root and - // all its descendents to stay in the tracker. - LeakReferenceAndIgnore(leaked); - - PASS(); -} - -// A scriptable object that should cause a crash if its destructor is run. We -// don't run the destructor for objects which the plugin leaks. This is to -// prevent them doing dangerous things at cleanup time, such as executing script -// or creating new objects. -class BadDestructorObject : public pp::deprecated::ScriptableObject { - public: - BadDestructorObject() {} - ~BadDestructorObject() { - assert(false); - } -}; - -std::string TestInstance::TestLeakedObjectDestructors() { - pp::VarPrivate leaked(instance(), new BadDestructorObject()); - // Leak a reference so it gets deleted on instance shutdown. - LeakReferenceAndIgnore(leaked); - PASS(); -} - diff --git a/ppapi/tests/test_instance_deprecated.h b/ppapi/tests/test_instance_deprecated.h index 6ec9b46..8b81501 100644 --- a/ppapi/tests/test_instance_deprecated.h +++ b/ppapi/tests/test_instance_deprecated.h @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -20,19 +20,12 @@ class TestInstance : public TestCase { void set_string(const std::string& s) { string_ = s; } - // Leak a reference to the given var, but ignore the leak in the leak checking - // that happens at shutdown. This allows us to test the "ForceFree" that - // happens on instance shutdown. - void LeakReferenceAndIgnore(const pp::Var& leaked); - protected: // Test case protected overrides. virtual pp::deprecated::ScriptableObject* CreateTestObject(); private: std::string TestExecuteScript(); - std::string TestRecursiveObjects(); - std::string TestLeakedObjectDestructors(); // Value written by set_string which is called by the ScriptableObject. This // allows us to keep track of what was called. diff --git a/ppapi/tests/testing_instance.cc b/ppapi/tests/testing_instance.cc index 1da6105..58e0890 100644 --- a/ppapi/tests/testing_instance.cc +++ b/ppapi/tests/testing_instance.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -102,16 +102,18 @@ bool TestingInstance::HandleInputEvent(const pp::InputEvent& event) { } void TestingInstance::EvalScript(const std::string& script) { - SendTestCommand("EvalScript", script); + std::string message("TESTING_MESSAGE:EvalScript:"); + message.append(script); + PostMessage(pp::Var(message)); } void TestingInstance::SetCookie(const std::string& name, const std::string& value) { - SendTestCommand("SetCookie", name + "=" + value); -} - -void TestingInstance::AddPostCondition(const std::string& script) { - SendTestCommand("AddPostCondition", script); + std::string message("TESTING_MESSAGE:SetCookie:"); + message.append(name); + message.append("="); + message.append(value); + PostMessage(pp::Var(message)); } void TestingInstance::LogTest(const std::string& test_name, @@ -148,7 +150,7 @@ void TestingInstance::ExecuteTests(int32_t unused) { ReportProgress(kProgressSignal); // Clear the console. - SendTestCommand("ClearConsole"); + PostMessage(pp::Var("TESTING_MESSAGE:ClearConsole")); if (!errors_.empty()) { // Catch initialization errors and output the current error string to @@ -167,9 +169,7 @@ 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_); - SendTestCommand("DidExecuteTests"); - // Note, DidExecuteTests unloads the plugin. We can't really do anthing after - // this point. + PostMessage(pp::Var("TESTING_MESSAGE:DidExecuteTests")); } TestCase* TestingInstance::CaseForTestName(const std::string& name) { @@ -190,18 +190,6 @@ std::string TestingInstance::FilterForTestName(const std::string& name) { return ""; } -void TestingInstance::SendTestCommand(const std::string& command) { - std::string msg("TESTING_MESSAGE:"); - msg += command; - PostMessage(pp::Var(msg)); -} - -void TestingInstance::SendTestCommand(const std::string& command, - const std::string& params) { - SendTestCommand(command + ":" + params); -} - - void TestingInstance::LogAvailableTests() { // Print out a listing of all tests. std::vector<std::string> test_cases; @@ -238,7 +226,9 @@ void TestingInstance::LogError(const std::string& text) { } void TestingInstance::LogHTML(const std::string& html) { - SendTestCommand("LogHTML", html); + std::string message("TESTING_MESSAGE:LogHTML:"); + message.append(html); + PostMessage(pp::Var(message)); } void TestingInstance::ReportProgress(const std::string& progress_value) { diff --git a/ppapi/tests/testing_instance.h b/ppapi/tests/testing_instance.h index f480e9d..7b05174 100644 --- a/ppapi/tests/testing_instance.h +++ b/ppapi/tests/testing_instance.h @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -89,11 +89,6 @@ pp::InstancePrivate { // Sets the given cookie in the current document. void SetCookie(const std::string& name, const std::string& value); - // Add a post-condition to the JavaScript on the test_case.html page. This - // JavaScript code will be run after the instance is shut down and must - // evaluate to |true| or the test will fail. - void AddPostCondition(const std::string& script); - private: void ExecuteTests(int32_t unused); @@ -111,10 +106,6 @@ pp::InstancePrivate { // Runs 'PostMessage_SendingData. std::string FilterForTestName(const std::string& name); - // Sends a test command to the page using PostMessage. - void SendTestCommand(const std::string& command); - void SendTestCommand(const std::string& command, const std::string& params); - // Appends a list of available tests to the console in the document. void LogAvailableTests(); diff --git a/webkit/plugins/ppapi/host_var_tracker.cc b/webkit/plugins/ppapi/host_var_tracker.cc index 57bc92c..51ba0e84 100644 --- a/webkit/plugins/ppapi/host_var_tracker.cc +++ b/webkit/plugins/ppapi/host_var_tracker.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -42,7 +42,7 @@ void HostVarTracker::AddNPObjectVar(NPObjectVar* object_var) { DCHECK(np_object_map->find(object_var->np_object()) == np_object_map->end()) << "NPObjectVar already in map"; np_object_map->insert( - std::make_pair(object_var->np_object(), object_var->AsWeakPtr())); + std::make_pair(object_var->np_object(), object_var)); } void HostVarTracker::RemoveNPObjectVar(NPObjectVar* object_var) { @@ -98,17 +98,14 @@ void HostVarTracker::ForceFreeNPObjectsForInstance(PP_Instance instance) { return; // Nothing to do. NPObjectToNPObjectVarMap* np_object_map = found_instance->second.get(); - // Force delete all var references. It's possible that deleting an object "A" - // will cause it to delete another object "B" it references, thus removing "B" - // from instance_map_. Therefore, we need to make a copy over which we can - // iterate safely. Furthermore, the maps contain WeakPtrs so that we can - // detect if the object is gone so that we don't dereference invalid memory. + // Force delete all var references. Need to make a copy so we can iterate over + // the map while deleting stuff from it. NPObjectToNPObjectVarMap np_object_map_copy = *np_object_map; NPObjectToNPObjectVarMap::iterator cur_var = np_object_map_copy.begin(); while (cur_var != np_object_map_copy.end()) { NPObjectToNPObjectVarMap::iterator current = cur_var++; - ForceReleaseNPObject(current->second); + current->second->InstanceDeleted(); np_object_map->erase(current->first); } @@ -117,23 +114,5 @@ void HostVarTracker::ForceFreeNPObjectsForInstance(PP_Instance instance) { instance_map_.erase(found_instance); } -void HostVarTracker::ForceReleaseNPObject( - const base::WeakPtr< ::ppapi::NPObjectVar>& object) { - // There's a chance that the object was already deleted before we got here. - // See ForceFreeNPObjectsForInstance for further explanation. If the object - // was deleted, the WeakPtr will return NULL. - if (!object.get()) - return; - object->InstanceDeleted(); - VarMap::iterator iter = live_vars_.find(object->GetExistingVarID()); - if (iter == live_vars_.end()) { - NOTREACHED(); - return; - } - iter->second.ref_count = 0; - DCHECK(iter->second.track_with_no_reference_count == 0); - DeleteObjectInfoIfNecessary(iter); -} - } // namespace ppapi } // namespace webkit diff --git a/webkit/plugins/ppapi/host_var_tracker.h b/webkit/plugins/ppapi/host_var_tracker.h index ece0659..fb743fc 100644 --- a/webkit/plugins/ppapi/host_var_tracker.h +++ b/webkit/plugins/ppapi/host_var_tracker.h @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -14,7 +14,6 @@ #include "base/memory/linked_ptr.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "base/memory/weak_ptr.h" #include "ppapi/c/pp_instance.h" #include "ppapi/c/pp_resource.h" #include "ppapi/shared_impl/function_group_base.h" @@ -67,12 +66,7 @@ class HostVarTracker : public ::ppapi::VarTracker { virtual ::ppapi::ArrayBufferVar* CreateArrayBuffer( uint32 size_in_bytes) OVERRIDE; - // Clear the reference count of the given object and remove it from - // live_vars_. - void ForceReleaseNPObject(const base::WeakPtr< ::ppapi::NPObjectVar>& object); - - typedef std::map<NPObject*, base::WeakPtr< ::ppapi::NPObjectVar> > - NPObjectToNPObjectVarMap; + typedef std::map<NPObject*, ::ppapi::NPObjectVar*> NPObjectToNPObjectVarMap; // Lists all known NPObjects, first indexed by the corresponding instance, // then by the NPObject*. This allows us to look up an NPObjectVar given diff --git a/webkit/plugins/ppapi/message_channel.cc b/webkit/plugins/ppapi/message_channel.cc index f02871d..f139114 100644 --- a/webkit/plugins/ppapi/message_channel.cc +++ b/webkit/plugins/ppapi/message_channel.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -412,8 +412,7 @@ MessageChannel::~MessageChannel() { void MessageChannel::SetPassthroughObject(NPObject* passthrough) { // Retain the passthrough object; We need to ensure it lives as long as this // MessageChannel. - if (passthrough) - WebBindings::retainObject(passthrough); + WebBindings::retainObject(passthrough); // If we had a passthrough set already, release it. Note that we retain the // incoming passthrough object first, so that we behave correctly if anyone diff --git a/webkit/plugins/ppapi/npobject_var.h b/webkit/plugins/ppapi/npobject_var.h index 1a187ac..b4e4269 100644 --- a/webkit/plugins/ppapi/npobject_var.h +++ b/webkit/plugins/ppapi/npobject_var.h @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -8,7 +8,6 @@ #include <string> #include "base/compiler_specific.h" -#include "base/memory/weak_ptr.h" #include "ppapi/c/pp_instance.h" #include "ppapi/shared_impl/var.h" #include "webkit/plugins/webkit_plugins_export.h" @@ -28,8 +27,7 @@ namespace ppapi { // PP_Var IDs) for each module. This allows us to track all references owned by // a given module and free them when the plugin exits independently of other // plugins that may be running at the same time. -class NPObjectVar : public Var, - public base::SupportsWeakPtr<NPObjectVar> { +class NPObjectVar : public Var { public: // You should always use FromNPObject to create an NPObjectVar. This function // guarantees that we maintain the 1:1 mapping between NPObject and diff --git a/webkit/plugins/ppapi/ppapi_plugin_instance.cc b/webkit/plugins/ppapi/ppapi_plugin_instance.cc index bed914c..1b6ff94 100644 --- a/webkit/plugins/ppapi/ppapi_plugin_instance.cc +++ b/webkit/plugins/ppapi/ppapi_plugin_instance.cc @@ -353,11 +353,6 @@ PluginInstance::~PluginInstance() { void PluginInstance::Delete() { // Keep a reference on the stack. See NOTE above. scoped_refptr<PluginInstance> ref(this); - // Force the MessageChannel to release its "passthrough object" which should - // release our last reference to the "InstanceObject" and will probably - // destroy it. We want to do this prior to calling DidDestroy in case the - // destructor of the instance object tries to use the instance. - message_channel_->SetPassthroughObject(NULL); instance_interface_->DidDestroy(pp_instance()); if (fullscreen_container_) { diff --git a/webkit/plugins/ppapi/ppapi_webplugin_impl.cc b/webkit/plugins/ppapi/ppapi_webplugin_impl.cc index fb01662..c505741 100644 --- a/webkit/plugins/ppapi/ppapi_webplugin_impl.cc +++ b/webkit/plugins/ppapi/ppapi_webplugin_impl.cc @@ -8,8 +8,7 @@ #include "base/message_loop.h" #include "googleurl/src/gurl.h" -#include "ppapi/shared_impl/ppapi_globals.h" -#include "ppapi/shared_impl/var_tracker.h" +#include "ppapi/c/pp_var.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebBindings.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebDocument.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebElement.h" @@ -52,8 +51,7 @@ WebPluginImpl::WebPluginImpl( const WebPluginParams& params, const base::WeakPtr<PluginDelegate>& plugin_delegate) : init_data_(new InitData()), - full_frame_(params.loadManually), - instance_object_(PP_MakeUndefined()) { + full_frame_(params.loadManually) { DCHECK(plugin_module); init_data_->module = plugin_module; init_data_->delegate = plugin_delegate; @@ -93,8 +91,6 @@ bool WebPluginImpl::initialize(WebPluginContainer* container) { void WebPluginImpl::destroy() { if (instance_) { - ::ppapi::PpapiGlobals::Get()->GetVarTracker()->ReleaseVar(instance_object_); - instance_object_ = PP_MakeUndefined(); instance_->Delete(); instance_ = NULL; } @@ -103,16 +99,17 @@ void WebPluginImpl::destroy() { } NPObject* WebPluginImpl::scriptableObject() { - // Call through the plugin to get its instance object. The plugin should pass - // us a reference which we release in destroy(). - if (instance_object_.type == PP_VARTYPE_UNDEFINED) - instance_object_ = instance_->GetInstanceObject(); + // Call through the plugin to get its instance object. Note that we "leak" a + // reference here. But we want to keep the instance object alive so long as + // the instance is alive, so it's okay. It will get cleaned up when all + // NPObjectVars are "force freed" at instance shutdown. + scoped_refptr<NPObjectVar> object( + NPObjectVar::FromPPVar(instance_->GetInstanceObject())); // GetInstanceObject talked to the plugin which may have removed the instance // from the DOM, in which case instance_ would be NULL now. if (!instance_) return NULL; - scoped_refptr<NPObjectVar> object(NPObjectVar::FromPPVar(instance_object_)); // If there's an InstanceObject, tell the Instance's MessageChannel to pass // any non-postMessage calls to it. if (object) { diff --git a/webkit/plugins/ppapi/ppapi_webplugin_impl.h b/webkit/plugins/ppapi/ppapi_webplugin_impl.h index ae319a8..e853281 100644 --- a/webkit/plugins/ppapi/ppapi_webplugin_impl.h +++ b/webkit/plugins/ppapi/ppapi_webplugin_impl.h @@ -11,7 +11,6 @@ #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "base/message_loop_helpers.h" -#include "ppapi/c/pp_var.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebPlugin.h" #include "ui/gfx/rect.h" #include "webkit/plugins/webkit_plugins_export.h" @@ -94,7 +93,6 @@ class WebPluginImpl : public WebKit::WebPlugin { scoped_refptr<PluginInstance> instance_; scoped_refptr<PPB_URLLoader_Impl> document_loader_; gfx::Rect plugin_rect_; - PP_Var instance_object_; DISALLOW_COPY_AND_ASSIGN(WebPluginImpl); }; |