PPAPI: Really force-free NPObjects on Instance destruction.

chrome/test/ui/ppapi_uitest.cc

Index: chrome/test/ui/ppapi_uitest.cc
diff --git a/chrome/test/ui/ppapi_uitest.cc b/chrome/test/ui/ppapi_uitest.cc
index e6fa0c95e8a223e2460440815887aa1469ca7087..1327d5e3564f5f872332e1aea828bf9394538f85 100644
--- a/chrome/test/ui/ppapi_uitest.cc
+++ b/chrome/test/ui/ppapi_uitest.cc
@@ -70,12 +70,40 @@ class PPAPITestBase : public UITest {
 
   void RunTest(const std::string& test_case) {
     scoped_refptr<TabProxy> tab = GetActiveTab();
-    EXPECT_TRUE(tab.get());
-    if (!tab.get())
-      return;
+    ASSERT_TRUE(tab.get());
+    GURL url = GetTestFileUrl(test_case);
+    EXPECT_TRUE(tab->NavigateToURLBlockUntilNavigationsComplete(url, 1));
+    RunTestURL(tab, url);
+  }
+
+  void RunTestAndReload(const std::string& test_case) {
+    scoped_refptr<TabProxy> tab = GetActiveTab();
+    ASSERT_TRUE(tab.get());
     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) {
@@ -275,6 +303,17 @@ class PPAPINaClTestDisallowedSockets : public PPAPITestBase {
     TEST_F(OutOfProcessPPAPITest, test_name) { \
       RunTest(STRIP_PREFIXES(test_name)); \
     }
+// Run the test and reload. This can test for clean shutdown, including leaked
+// vars.
+#define TEST_PPAPI_IN_PROCESS_AND_RELOAD(test_name) \
+    TEST_F(PPAPITest, test_name) { \
+      RunTestAndReload(STRIP_PREFIXES(test_name)); \
+    }
+#define TEST_PPAPI_OUT_OF_PROCESS_AND_RELOAD(test_name) \
+    TEST_F(OutOfProcessPPAPITest, test_name) { \
+      RunTestAndReload(STRIP_PREFIXES(test_name)); \
+    }
+
 
 // Similar macros that test over HTTP.
 #define TEST_PPAPI_IN_PROCESS_VIA_HTTP(test_name) \
@@ -365,7 +404,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)
+// We run and reload the InstanceDeprecated test to ensure that the
+// InstanceObject is properly cleaned up after the first run.
+TEST_PPAPI_IN_PROCESS_AND_RELOAD(Instance)

[brettw, 2012/02/16 23:40:45]

I think the macros are getting out of control. Do you think we'll be using this
mode on other tests? I'd say just code up the behavior you want for this test.

+// TODO(dmichael): Fix the similar leak for o-o-p and use the RELOAD test.
 TEST_PPAPI_OUT_OF_PROCESS(Instance)
 // The Instance test is actually InstanceDeprecated which isn't supported
 // by NaCl.