Make a BrowserPluginSpecific test a MimeHandlerViewTest
All GuestViews except for MimeHandlerViewGuest are now implemented based on
cross-process frames. There is however a hadnful of browser tests which are not
MimeHandlerViewTest but still use BrowserPlugin.
This CL will
1- Change the MimeHandlerViewTest from parameteric to normal test.
Essentially, the UseCrossProcessFramesForGuest flag is unrelated to
the MimeHandlerViewGuest. There is a separate flag for that matter
which is "MimeHandlerViewInCrossProcessFrame" and will be added
eventually when the a first version of MimeHandlerViewGuest based on
cross-process frames is implemented on ToT.
2- Move WebViewBrowserPluginSpecificTest.AcceptTouchEvents to
MimeHandlerViewTest. This involves overhauling the test test is also
modified to load MimeHandlerViewGuest instead of WebViewGuest.
The short-term plan is to change all such (BrowserPluginSpecific) tests
to become MimeHandlerViewBrowserPluginSpecificTest. The longer term goal
is to remove all such tests when MimeHandlerViewInCrossProcessFrames
is shipped.
Bug: 659750, 533069, 330264
Change-Id: Icad9ddd1f2d1e6851b20f6c0d923dd2192b7dcec
Reviewed-on: https://chromium-review.googlesource.com/1161745
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580655}diff --git a/chrome/browser/apps/guest_view/web_view_browsertest.cc b/chrome/browser/apps/guest_view/web_view_browsertest.cc
index a874c0e..9183faa 100644
--- a/chrome/browser/apps/guest_view/web_view_browsertest.cc
+++ b/chrome/browser/apps/guest_view/web_view_browsertest.cc
@@ -1143,41 +1143,6 @@
ASSERT_TRUE(launched_again_listener.WaitUntilSatisfied());
}
-IN_PROC_BROWSER_TEST_F(WebViewBrowserPluginSpecificTest, AcceptTouchEvents) {
- // This test only makes sense for non-OOPIF WebView, since with
- // GuestViewCrossProcessFrames events are routed directly to the
- // guest, so the embedder does not need to know about the installation of
- // touch handlers.
- ASSERT_FALSE(
- base::FeatureList::IsEnabled(::features::kGuestViewCrossProcessFrames));
-
- LoadAppWithGuest("web_view/accept_touch_events");
-
- content::RenderViewHost* embedder_rvh =
- GetEmbedderWebContents()->GetRenderViewHost();
-
- bool embedder_has_touch_handler =
- content::RenderViewHostTester::HasTouchEventHandler(embedder_rvh);
- EXPECT_FALSE(embedder_has_touch_handler);
-
- SendMessageToGuestAndWait("install-touch-handler", "installed-touch-handler");
-
- // Note that we need to wait for the installed/registered touch handler to
- // appear in browser process before querying |embedder_rvh|.
- // In practice, since we do a roundrtip from browser process to guest and
- // back, this is sufficient.
- embedder_has_touch_handler =
- content::RenderViewHostTester::HasTouchEventHandler(embedder_rvh);
- EXPECT_TRUE(embedder_has_touch_handler);
-
- SendMessageToGuestAndWait("uninstall-touch-handler",
- "uninstalled-touch-handler");
- // Same as the note above about waiting.
- embedder_has_touch_handler =
- content::RenderViewHostTester::HasTouchEventHandler(embedder_rvh);
- EXPECT_FALSE(embedder_has_touch_handler);
-}
-
// This test ensures JavaScript errors ("Cannot redefine property") do not
// happen when a <webview> is removed from DOM and added back.
IN_PROC_BROWSER_TEST_F(WebViewTest, AddRemoveWebView_AddRemoveWebView) {
diff --git a/chrome/test/data/extensions/api_test/mime_handler_view/index.js b/chrome/test/data/extensions/api_test/mime_handler_view/index.js
index 72d5e7c9..ec1f767 100644
--- a/chrome/test/data/extensions/api_test/mime_handler_view/index.js
+++ b/chrome/test/data/extensions/api_test/mime_handler_view/index.js
@@ -58,6 +58,10 @@
chrome.test.assertTrue(streamDetails.tabId != -1);
}
+// The following helper methods are used in BrowserPlugin-specific tests.
+function dummyTouchStartHandler(e) {
+}
+
var tests = [
function testBasic() {
checkStreamDetails('testBasic.csv', false);
diff --git a/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc b/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc
index 292e3a7..99a6758 100644
--- a/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc
+++ b/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc
@@ -10,6 +10,8 @@
#include "base/path_service.h"
#include "base/run_loop.h"
#include "base/test/scoped_feature_list.h"
+#include "base/test/test_timeouts.h"
+#include "base/threading/thread_task_runner_handle.h"
#include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/test/base/ui_test_utils.h"
@@ -19,6 +21,7 @@
#include "content/public/common/content_features.h"
#include "content/public/common/content_switches.h"
#include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_renderer_host.h"
#include "extensions/browser/api/extensions_api_client.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/guest_view/extensions_guest_view_manager_delegate.h"
@@ -41,8 +44,7 @@
// The test extension id is set by the key value in the manifest.
const char kExtensionId[] = "oickdpebdnfbgkcaoklfcdhjniefkcji";
-class MimeHandlerViewTest : public extensions::ExtensionApiTest,
- public testing::WithParamInterface<bool> {
+class MimeHandlerViewTest : public extensions::ExtensionApiTest {
public:
MimeHandlerViewTest() {
GuestViewManager::set_factory_for_testing(&factory_);
@@ -60,24 +62,6 @@
ASSERT_TRUE(StartEmbeddedTestServer());
}
- // TODO(ekaramad): These tests run for OOPIF guests too, except that they
- // still use BrowserPlugin code path. They are activated to make sure we can
- // still show PDF when the rest of the guests migrate to OOPIF. Eventually,
- // MimeHandlerViewGuest will be based on OOPIF and we can remove this comment
- // (https://crbug.com/642826).
- void SetUpCommandLine(base::CommandLine* command_line) override {
- extensions::ExtensionApiTest::SetUpCommandLine(command_line);
-
- bool use_cross_process_frames_for_guests = GetParam();
- if (use_cross_process_frames_for_guests) {
- scoped_feature_list_.InitAndEnableFeature(
- features::kGuestViewCrossProcessFrames);
- } else {
- scoped_feature_list_.InitAndDisableFeature(
- features::kGuestViewCrossProcessFrames);
- }
- }
-
// TODO(paulmeyer): This function is implemented over and over by the
// different GuestView test classes. It really needs to be refactored out to
// some kind of GuestViewTest base class.
@@ -142,27 +126,23 @@
int basic_count_ = 0;
};
-INSTANTIATE_TEST_CASE_P(MimeHandlerViewTests,
- MimeHandlerViewTest,
- testing::Bool());
-
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, PostMessage) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, PostMessage) {
RunTest("test_postmessage.html");
}
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, Basic) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, Basic) {
RunTest("testBasic.csv");
}
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, Embedded) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, Embedded) {
RunTest("test_embedded.html");
}
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, Iframe) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, Iframe) {
RunTest("test_iframe.html");
}
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, Abort) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, Abort) {
if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
// With the network service, abortStream isn't needed since we pass a Mojo
// pipe to the renderer. If the plugin chooses to cancel the main request
@@ -177,28 +157,28 @@
RunTest("testAbort.csv");
}
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, NonAsciiHeaders) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, NonAsciiHeaders) {
RunTest("testNonAsciiHeaders.csv");
}
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, DataUrl) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, DataUrl) {
const char* kDataUrlCsv = "data:text/csv;base64,Y29udGVudCB0byByZWFkCg==";
RunTestWithUrl(GURL(kDataUrlCsv));
}
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, EmbeddedDataUrlObject) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, EmbeddedDataUrlObject) {
RunTest("test_embedded_data_url_object.html");
}
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, EmbeddedDataUrlEmbed) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, EmbeddedDataUrlEmbed) {
RunTest("test_embedded_data_url_embed.html");
}
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, EmbeddedDataUrlLong) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, EmbeddedDataUrlLong) {
RunTest("test_embedded_data_url_long.html");
}
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, ResizeBeforeAttach) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, ResizeBeforeAttach) {
// Delay the creation of the guest's WebContents in order to delay the guest's
// attachment to the embedder. This will allow us to resize the <object> tag
// after the guest is created, but before it is attached in
@@ -220,20 +200,20 @@
}
// Regression test for crbug.com/587709.
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, SingleRequest) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, SingleRequest) {
GURL url(embedded_test_server()->GetURL("/testBasic.csv"));
RunTest("testBasic.csv");
EXPECT_EQ(1, basic_count());
}
// Test that a mime handler view can keep a background page alive.
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, BackgroundPage) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, BackgroundPage) {
extensions::ProcessManager::SetEventPageIdleTimeForTesting(1);
extensions::ProcessManager::SetEventPageSuspendingTimeForTesting(1);
RunTest("testBackgroundPage.csv");
}
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, TargetBlankAnchor) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, TargetBlankAnchor) {
RunTest("testTargetBlankAnchor.csv");
ASSERT_EQ(2, browser()->tab_strip_model()->count());
content::WaitForLoadStop(browser()->tab_strip_model()->GetWebContentsAt(1));
@@ -242,7 +222,7 @@
browser()->tab_strip_model()->GetWebContentsAt(1)->GetLastCommittedURL());
}
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, BeforeUnload_NoDialog) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, BeforeUnload_NoDialog) {
ASSERT_NO_FATAL_FAILURE(RunTest("testBeforeUnloadNoDialog.csv"));
auto* web_contents = browser()->tab_strip_model()->GetWebContentsAt(0);
content::PrepContentsForBeforeUnloadTest(web_contents);
@@ -257,7 +237,7 @@
ui_test_utils::NavigateToURL(browser(), GURL("about:blank"));
}
-IN_PROC_BROWSER_TEST_P(MimeHandlerViewTest, BeforeUnload_ShowDialog) {
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewTest, BeforeUnload_ShowDialog) {
ASSERT_NO_FATAL_FAILURE(RunTest("testBeforeUnloadShowDialog.csv"));
auto* web_contents = browser()->tab_strip_model()->GetWebContentsAt(0);
content::PrepContentsForBeforeUnloadTest(web_contents);
@@ -275,3 +255,58 @@
EXPECT_FALSE(before_unload_dialog->is_reload());
before_unload_dialog->OnAccept(base::string16(), false);
}
+
+// TODO(mcnee): These tests are BrowserPlugin specific. Once
+// MimeHandlerViewGuest is no longer based on BrowserPlugin, remove these tests.
+// (See https://crbug.com/533069 and https://crbug.com/659750). These category
+// of tests are solely testing BrowserPlugin features.
+class MimeHandlerViewBrowserPluginSpecificTest : public MimeHandlerViewTest {
+ public:
+ MimeHandlerViewBrowserPluginSpecificTest() {}
+
+ ~MimeHandlerViewBrowserPluginSpecificTest() override {}
+
+ protected:
+ // None of these test create new tabs, so the embedder should be the first
+ // tab.
+ content::WebContents* GetEmbedderWebContents() {
+ return browser()->tab_strip_model()->GetWebContentsAt(0);
+ }
+
+ DISALLOW_COPY_AND_ASSIGN(MimeHandlerViewBrowserPluginSpecificTest);
+};
+
+// This test verifies that when BrowserPlugin-based guest has touch handlers,
+// the embedder knows about it.
+IN_PROC_BROWSER_TEST_F(MimeHandlerViewBrowserPluginSpecificTest,
+ AcceptTouchEvents) {
+ RunTest("testBasic.csv");
+ content::RenderViewHost* embedder_rvh =
+ GetEmbedderWebContents()->GetRenderViewHost();
+ bool embedder_has_touch_handler =
+ content::RenderViewHostTester::HasTouchEventHandler(embedder_rvh);
+ EXPECT_FALSE(embedder_has_touch_handler);
+
+ auto* guest_web_contents = GetGuestViewManager()->WaitForSingleGuestCreated();
+ ASSERT_TRUE(ExecuteScript(
+ guest_web_contents,
+ "document.addEventListener('touchstart', dummyTouchStartHandler);"));
+ // Wait until embedder has touch handlers.
+ while (!content::RenderViewHostTester::HasTouchEventHandler(embedder_rvh)) {
+ base::RunLoop run_loop;
+ base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+ FROM_HERE, run_loop.QuitClosure(), TestTimeouts::tiny_timeout());
+ run_loop.Run();
+ }
+
+ ASSERT_TRUE(ExecuteScript(
+ guest_web_contents,
+ "document.removeEventListener('touchstart', dummyTouchStartHandler);"));
+ // Wait until embedder not longer has any touch handlers.
+ while (content::RenderViewHostTester::HasTouchEventHandler(embedder_rvh)) {
+ base::RunLoop run_loop;
+ base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+ FROM_HERE, run_loop.QuitClosure(), TestTimeouts::tiny_timeout());
+ run_loop.Run();
+ }
+}
\ No newline at end of file
