Commit d2cae304 authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Fire NOTIFICATION_EXTENSION_HOST_CREATED at most once per ExtensionHost.

Tests assume that NOTIFICATION_EXTENSION_HOST_CREATED fires at most once
(see BrowserActionInteractiveTest::TearDownOnMainThread which compares
the number of "created" vs "destroyed" notifications).  This assumption
has not been met by the product code, which would fire
NOTIFICATION_EXTENSION_HOST_CREATED for every RenderViewReady
notification which happens not only for the very first RenderViewHost,
but also for possible subsequent RenderViewHost swaps (e.g. when
navigating cross-site).

This CL fixes ExtensionHost::RenderViewReady, to ensure that the
NOTIFICATION_EXTENSION_HOST_CREATED fires at most once per ExtensionHost
object.

Note that NOTIFICATION_EXTENSION_HOST_CREATED is only listened to by
test code (browser_action_interactive_test.cc and
management_api_browsertest.cc).  This means that this CL should have no
effect on production code and product behavior.

Fixed: 1066287
Change-Id: I30b26294c9edb18420b7d3d4b8d560c360914a06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2129230
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarKaran Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755504}
parent 5fe43d77
...@@ -886,19 +886,13 @@ IN_PROC_BROWSER_TEST_F(NavigatingExtensionPopupInteractiveTest, ...@@ -886,19 +886,13 @@ IN_PROC_BROWSER_TEST_F(NavigatingExtensionPopupInteractiveTest,
// Tests that an extension pop-up cannot be navigated to a page // Tests that an extension pop-up cannot be navigated to a page
// in another extension. // in another extension.
//
// TODO(lukasza): https://crbug.com/1066287: Enable the two tests below after
// fixing ExtensionHost to ensure that NOTIFICATION_EXTENSION_HOST_CREATED fires
// at most once - otherwise the test assertion below would fail in
// TearDownOnMainThread:
// EXPECT_EQ(host_watcher_->created(), host_watcher_->destroyed());
IN_PROC_BROWSER_TEST_F(NavigatingExtensionPopupInteractiveTest, IN_PROC_BROWSER_TEST_F(NavigatingExtensionPopupInteractiveTest,
DISABLED_PageInOtherExtension_Get) { PageInOtherExtension_Get) {
GURL other_extension_url = other_extension().GetResourceURL("other.html"); GURL other_extension_url = other_extension().GetResourceURL("other.html");
TestPopupNavigationViaGet(other_extension_url, EXPECTING_NAVIGATION_FAILURE); TestPopupNavigationViaGet(other_extension_url, EXPECTING_NAVIGATION_FAILURE);
} }
IN_PROC_BROWSER_TEST_F(NavigatingExtensionPopupInteractiveTest, IN_PROC_BROWSER_TEST_F(NavigatingExtensionPopupInteractiveTest,
DISABLED_PageInOtherExtension_Post) { PageInOtherExtension_Post) {
GURL other_extension_url = other_extension().GetResourceURL("other.html"); GURL other_extension_url = other_extension().GetResourceURL("other.html");
TestPopupNavigationViaPost(other_extension_url, EXPECTING_NAVIGATION_FAILURE); TestPopupNavigationViaPost(other_extension_url, EXPECTING_NAVIGATION_FAILURE);
} }
......
...@@ -416,6 +416,10 @@ void ExtensionHost::AddNewContents(WebContents* source, ...@@ -416,6 +416,10 @@ void ExtensionHost::AddNewContents(WebContents* source,
} }
void ExtensionHost::RenderViewReady() { void ExtensionHost::RenderViewReady() {
if (has_creation_notification_already_fired_)
return;
has_creation_notification_already_fired_ = true;
content::NotificationService::current()->Notify( content::NotificationService::current()->Notify(
extensions::NOTIFICATION_EXTENSION_HOST_CREATED, extensions::NOTIFICATION_EXTENSION_HOST_CREATED,
content::Source<BrowserContext>(browser_context_), content::Source<BrowserContext>(browser_context_),
......
...@@ -186,6 +186,12 @@ class ExtensionHost : public DeferredStartRenderHost, ...@@ -186,6 +186,12 @@ class ExtensionHost : public DeferredStartRenderHost,
// Whether CreateRenderViewNow was called before the extension was ready. // Whether CreateRenderViewNow was called before the extension was ready.
bool is_render_view_creation_pending_; bool is_render_view_creation_pending_;
// Whether NOTIFICATION_EXTENSION_HOST_CREATED has been already delivered
// (since it is triggered by RenderViewReady which happens not only for the
// very first RenderViewHost, but also can happen when swapping RenderViewHost
// for another one).
bool has_creation_notification_already_fired_ = false;
// Whether the ExtensionHost has finished loading some content at least once. // Whether the ExtensionHost has finished loading some content at least once.
// There may be subsequent loads - such as reloads and navigations - and this // There may be subsequent loads - such as reloads and navigations - and this
// will not affect its value (it will remain true). // will not affect its value (it will remain true).
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment