Commit 4a56010a authored by W. James MacLean's avatar W. James MacLean Committed by Commit Bot

Delete MimeHandlerViewFrameContainerManager when no containers left and embedded.

When the last MimeHandlerViewFrameContainer has been removed from a
manager, and it's serving an embedded MimeHandlerView (i.e. not a full
page MimeHandlerView), delete the manager by removing it from the map.

Bug: 1005992, 1004804
Change-Id: I445b13fd33ea029adbbb256cffb572d3078b1d52
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1829652Reviewed-by: default avatarWei Li <weili@chromium.org>
Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Reviewed-by: default avatarFinnur Thorarinsson <finnur@chromium.org>
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703858}
parent b8c09924
...@@ -296,6 +296,7 @@ source_set("chrome_extensions_browsertests") { ...@@ -296,6 +296,7 @@ source_set("chrome_extensions_browsertests") {
"//components/dom_distiller/content/browser", "//components/dom_distiller/content/browser",
"//components/dom_distiller/core:test_support", "//components/dom_distiller/core:test_support",
"//components/guest_view/browser:test_support", "//components/guest_view/browser:test_support",
"//components/printing/common",
"//components/resources", "//components/resources",
"//components/strings", "//components/strings",
"//components/sync", "//components/sync",
......
...@@ -40,4 +40,7 @@ specific_include_rules = { ...@@ -40,4 +40,7 @@ specific_include_rules = {
".*(test|test_util)\.(cc|h)$": [ ".*(test|test_util)\.(cc|h)$": [
"+content/public/test", "+content/public/test",
], ],
"mime_handler_view_browsertest.cc": [
"+components/printing/common",
],
} }
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include "components/app_modal/javascript_app_modal_dialog.h" #include "components/app_modal/javascript_app_modal_dialog.h"
#include "components/app_modal/native_app_modal_dialog.h" #include "components/app_modal/native_app_modal_dialog.h"
#include "components/guest_view/browser/test_guest_view_manager.h" #include "components/guest_view/browser/test_guest_view_manager.h"
#include "components/printing/common/print_messages.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_handle.h" #include "content/public/browser/navigation_handle.h"
#include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_observer.h"
...@@ -220,6 +221,60 @@ IN_PROC_BROWSER_TEST_P(MimeHandlerViewCrossProcessTest, Embedded) { ...@@ -220,6 +221,60 @@ IN_PROC_BROWSER_TEST_P(MimeHandlerViewCrossProcessTest, Embedded) {
EXPECT_EQ(1U, gv_manager->num_guests_created()); EXPECT_EQ(1U, gv_manager->num_guests_created());
} }
#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
class PrintPreviewWaiter : public content::BrowserMessageFilter {
public:
PrintPreviewWaiter() : BrowserMessageFilter(PrintMsgStart) {}
bool OnMessageReceived(const IPC::Message& message) override {
IPC_BEGIN_MESSAGE_MAP(PrintPreviewWaiter, message)
IPC_MESSAGE_HANDLER(PrintHostMsg_DidStartPreview, OnDidStartPreview)
IPC_END_MESSAGE_MAP()
return false;
}
void OnDidStartPreview(const PrintHostMsg_DidStartPreview_Params& params,
const PrintHostMsg_PreviewIds& ids) {
// Expect that there is at least one page.
did_load_ = true;
run_loop_.Quit();
EXPECT_TRUE(params.page_count >= 1);
}
void Wait() {
if (!did_load_)
run_loop_.Run();
}
private:
~PrintPreviewWaiter() override = default;
bool did_load_ = false;
base::RunLoop run_loop_;
};
IN_PROC_BROWSER_TEST_P(MimeHandlerViewCrossProcessTest, EmbeddedThenPrint) {
RunTest("test_embedded.html");
ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
auto* gv_manager = GetGuestViewManager();
gv_manager->WaitForAllGuestsDeleted();
EXPECT_EQ(1U, gv_manager->num_guests_created());
// Verify that print dialog comes up.
auto* web_contents = browser()->tab_strip_model()->GetActiveWebContents();
auto* main_frame = web_contents->GetMainFrame();
auto print_preview_waiter = base::MakeRefCounted<PrintPreviewWaiter>();
web_contents->GetMainFrame()->GetProcess()->AddFilter(
print_preview_waiter.get());
// Use setTimeout() to prevent ExecuteScript() from blocking on the print
// dialog.
ASSERT_TRUE(content::ExecuteScript(
main_frame, "setTimeout(function() { window.print(); }, 0)"));
print_preview_waiter->Wait();
}
#endif // BUILDFLAG(ENABLE_PRINT_PREVIEW)
// This test start with an <object> that has a content frame. Then the content // This test start with an <object> that has a content frame. Then the content
// frame (plugin frame) is navigated to a cross-origin target page. After the // frame (plugin frame) is navigated to a cross-origin target page. After the
// navigation is completed, the <object> is set to render MimeHandlerView by // navigation is completed, the <object> is set to render MimeHandlerView by
......
...@@ -99,6 +99,8 @@ bool MimeHandlerViewContainerManager::CreateFrameContainer( ...@@ -99,6 +99,8 @@ bool MimeHandlerViewContainerManager::CreateFrameContainer(
// This should translate into a same document navigation. // This should translate into a same document navigation.
return true; return true;
} }
// Make sure we're not eligible to self-delete.
DCHECK(!internal_id_.empty());
// If there is already a MHVFC for this |plugin_element|, destroy it. // If there is already a MHVFC for this |plugin_element|, destroy it.
RemoveFrameContainerForReason(old_frame_container, RemoveFrameContainerForReason(old_frame_container,
UMAType::kRemoveFrameContainerUpdatePlugin); UMAType::kRemoveFrameContainerUpdatePlugin);
...@@ -268,6 +270,15 @@ bool MimeHandlerViewContainerManager::RemoveFrameContainer( ...@@ -268,6 +270,15 @@ bool MimeHandlerViewContainerManager::RemoveFrameContainer(
if (it == frame_containers_.cend()) if (it == frame_containers_.cend())
return false; return false;
frame_containers_.erase(it); frame_containers_.erase(it);
if (!frame_containers_.size() && internal_id_.empty()) {
// There are no frame containers left, and we're not serving a full-page
// MimeHandlerView, so we remove ourselves from the map.
auto& map = *GetRenderFrameMap();
map.erase(std::remove_if(map.begin(), map.end(), [this](const auto& iter) {
return iter.second.get() == this;
}));
}
return true; return true;
} }
......
...@@ -80,6 +80,8 @@ class MimeHandlerViewContainerManager ...@@ -80,6 +80,8 @@ class MimeHandlerViewContainerManager
v8::Isolate* isolate); v8::Isolate* isolate);
// Removes the |frame_container| from |frame_containers_| and destroys it. The // Removes the |frame_container| from |frame_containers_| and destroys it. The
// |reason| is emitted for UMA. // |reason| is emitted for UMA.
// Note: Calling this function may delete |this| if we are removing the last
// frame container.
void RemoveFrameContainerForReason( void RemoveFrameContainerForReason(
MimeHandlerViewFrameContainer* frame_container, MimeHandlerViewFrameContainer* frame_container,
MimeHandlerViewUMATypes::Type reason); MimeHandlerViewUMATypes::Type reason);
...@@ -100,6 +102,9 @@ class MimeHandlerViewContainerManager ...@@ -100,6 +102,9 @@ class MimeHandlerViewContainerManager
void LoadEmptyPage(const GURL& resource_url) override; void LoadEmptyPage(const GURL& resource_url) override;
void CreateBeforeUnloadControl( void CreateBeforeUnloadControl(
CreateBeforeUnloadControlCallback callback) override; CreateBeforeUnloadControlCallback callback) override;
// Note: Calling this function may delete |this| if we are destroying the last
// frame container.
void DestroyFrameContainer(int32_t element_instance_id) override; void DestroyFrameContainer(int32_t element_instance_id) override;
void DidLoad(int32_t mime_handler_view_guest_element_instance_id, void DidLoad(int32_t mime_handler_view_guest_element_instance_id,
const GURL& resource_url) override; const GURL& resource_url) override;
...@@ -111,6 +116,8 @@ class MimeHandlerViewContainerManager ...@@ -111,6 +116,8 @@ class MimeHandlerViewContainerManager
bool IsEmbedded() const override; bool IsEmbedded() const override;
bool IsResourceAccessibleBySource() const override; bool IsResourceAccessibleBySource() const override;
// Note: Calling this function may delete |this| if we are removing the last
// frame container.
bool RemoveFrameContainer(MimeHandlerViewFrameContainer* frame_container); bool RemoveFrameContainer(MimeHandlerViewFrameContainer* frame_container);
// mime_handler::BeforeUnloadControl implementation. // mime_handler::BeforeUnloadControl implementation.
void SetShowBeforeUnloadDialog( void SetShowBeforeUnloadDialog(
...@@ -137,7 +144,7 @@ class MimeHandlerViewContainerManager ...@@ -137,7 +144,7 @@ class MimeHandlerViewContainerManager
// Used to match against plugin elements that request a scriptable object. The // Used to match against plugin elements that request a scriptable object. The
// one that matches is the one inserted in the HTML string injected by the // one that matches is the one inserted in the HTML string injected by the
// MimeHandlerViewAttachHelper (and hence requires a scriptable object to for // MimeHandlerViewAttachHelper (and hence requires a scriptable object to for
// postMessage purposes). // postMessage purposes). This will only be non-empty for full-page MHV.
std::string internal_id_; std::string internal_id_;
// The plugin element that is managed by MimeHandlerViewContainerManager. // The plugin element that is managed by MimeHandlerViewContainerManager.
blink::WebElement plugin_element_; blink::WebElement plugin_element_;
......
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