Make ppapi TestView use default browser test timeouts rather than internal ones

ppapi/tests/test_view.cc

 // 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_view.h"
 
 #include <sstream>
 
 #include "ppapi/c/pp_time.h"
 #include "ppapi/c/private/ppb_testing_private.h"
@@ skipping 38 matching lines @@
   RUN_TEST(ScrollOffsetChange, filter);
 }
 
 bool TestView::WaitUntilViewChanged() {
   // Schedule a callback so this step times out if we don't get a ViewChanged
   // in a reasonable amount of time.
   pp::CompletionCallbackFactory<TestView> factory(this);
   pp::CompletionCallback timeout =
       factory.NewCallback(&TestView::QuitMessageLoop);
   pp::Module::Get()->core()->CallOnMainThread(
       kViewChangeTimeoutSec * 1000, timeout);

[benwells, 2015/01/28 02:55:50]

Can this be removed as well?

[dmichael (off chromium), 2015/01/28 17:50:32]

I'm not super familiar with this test, but the pattern here is to quit the
message loop if we haven't received the View event within 5 seconds.

With the code as it is in this CL, we'll just drop back in to the loop and try
again anyway. So there seems to be no point in this QuitMessageLoop event.

We can just rely on the test framework to kill the test if it hangs.

 
   size_t old_page_visibility_change_count = page_visibility_log_.size();
 
   // Run a nested message loop. It will exit either on ViewChanged or if the
   // timeout happens.
   post_quit_on_view_changed_ = true;
   testing_interface_->RunMessageLoop(instance_->pp_instance());
   post_quit_on_view_changed_ = false;
 
   // We know we got a view changed event if something was appended to the log.
@@ skipping 27 matching lines @@
 std::string TestView::TestPageHideShow() {
   // Initial state should be visible.
   ASSERT_FALSE(page_visibility_log_.empty());
   ASSERT_TRUE(page_visibility_log_[0]);
 
   // Now that we're alive, tell the test knows it can change our visibility.
   instance_->ReportProgress("TestPageHideShow:Created");
 
   // Wait until we get a hide event, being careful to handle spurious
   // notifications of ViewChanged.
-  PP_Time begin_time = pp::Module::Get()->core()->GetTime();
   while (WaitUntilViewChanged() &&
-         page_visibility_log_[page_visibility_log_.size() - 1] &&
-         pp::Module::Get()->core()->GetTime() - begin_time <
-             kViewChangeTimeoutSec) {
+         page_visibility_log_[page_visibility_log_.size() - 1]) {
   }
   if (page_visibility_log_[page_visibility_log_.size() - 1]) {
     // Didn't get a view changed event that changed visibility (though there
     // may have been some that didn't change visibility).
     // Add more error message since this test has some extra requirements.
     return "Didn't receive a hide event in timeout. NOTE: "
         "This test requires tab visibility to change and won't pass if you "
         "just run it in a browser. Normally the UI test should handle "
         "this. You can also run manually by waiting 2 secs, creating a new "
         "tab, waiting 2 more secs, and closing the new tab.";
   }
 
   // Tell the test so it can show us again.
   instance_->ReportProgress("TestPageHideShow:Hidden");
 
   // Wait until we get a show event.
-  begin_time = pp::Module::Get()->core()->GetTime();
   while (WaitUntilViewChanged() &&
-         !page_visibility_log_[page_visibility_log_.size() - 1] &&
-         pp::Module::Get()->core()->GetTime() - begin_time <
-             kViewChangeTimeoutSec) {
+         !page_visibility_log_[page_visibility_log_.size() - 1]) {
   }
   ASSERT_TRUE(page_visibility_log_[page_visibility_log_.size() - 1]);
 
   PASS();
 }
 
 std::string TestView::TestSizeChange() {
   pp::Rect original_rect = last_view_.GetRect();
 
   pp::Rect desired_rect = original_rect;
   desired_rect.set_width(original_rect.width() + 10);
   desired_rect.set_height(original_rect.height() + 12);
 
   std::ostringstream script_stream;
   script_stream << "var plugin = document.getElementById('plugin');";
   script_stream << "plugin.setAttribute('width', "
                 << desired_rect.width() << ");";
   script_stream << "plugin.setAttribute('height', "
                 << desired_rect.height() << ");";
 
   instance_->EvalScript(script_stream.str());
 
-  PP_Time begin_time = pp::Module::Get()->core()->GetTime();
-  while (WaitUntilViewChanged() && last_view_.GetRect() != desired_rect &&
-         pp::Module::Get()->core()->GetTime() - begin_time <
-             kViewChangeTimeoutSec) {
+  while (WaitUntilViewChanged() && last_view_.GetRect() != desired_rect) {
   }
   ASSERT_TRUE(last_view_.GetRect() == desired_rect);
 
   PASS();
 }
 
 std::string TestView::TestClipChange() {
   pp::Rect original_rect = last_view_.GetRect();
 
   // Original clip should be the full frame.
@@ skipping 16 matching lines @@
                                     "top:0px; width:1px; height:5000px;');"
       << "document.body.appendChild(big);"
       << "window.scrollBy(0, " << original_rect.y() + clip_amount << ");";
 
   instance_->EvalScript(script_stream.str());
 
   pp::Rect desired_clip = original_clip;
   desired_clip.set_y(clip_amount);
   desired_clip.set_height(desired_clip.height() - desired_clip.y());
 
-  PP_Time begin_time = pp::Module::Get()->core()->GetTime();
-  while (WaitUntilViewChanged() && last_view_.GetClipRect() != desired_clip &&
-         pp::Module::Get()->core()->GetTime() - begin_time <
-             kViewChangeTimeoutSec) {
+  while (WaitUntilViewChanged() && last_view_.GetClipRect() != desired_clip) {
   }
   ASSERT_TRUE(last_view_.GetClipRect() == desired_clip);
   PASS();
 }
 
 std::string TestView::TestScrollOffsetChange() {
   instance_->EvalScript("document.body.style.width = '5000px';"
                         "document.body.style.height = '5000px';");
   instance_->EvalScript("window.scrollTo(5, 1);");
 
-  PP_Time begin_time = pp::Module::Get()->core()->GetTime();
   while (WaitUntilViewChanged() &&
-         last_view_.GetScrollOffset() != pp::Point(5, 1) &&
-         pp::Module::Get()->core()->GetTime() - begin_time <
-             kViewChangeTimeoutSec) {
+         last_view_.GetScrollOffset() != pp::Point(5, 1)) {
   }
   ASSERT_EQ(pp::Point(5, 1), last_view_.GetScrollOffset());
 
   instance_->EvalScript("window.scrollTo(0, 0);");
 
-  begin_time = pp::Module::Get()->core()->GetTime();
   while (WaitUntilViewChanged() &&
-         last_view_.GetScrollOffset() != pp::Point(0, 0) &&
-         pp::Module::Get()->core()->GetTime() - begin_time <
-             kViewChangeTimeoutSec) {
+         last_view_.GetScrollOffset() != pp::Point(0, 0)) {
   }
   ASSERT_EQ(pp::Point(0, 0), last_view_.GetScrollOffset());
 
   PASS();
 }