Commit 0f90533b authored by creis's avatar creis Committed by Commit bot

Fix incorrect DCHECK in task manager's WebContentsEntry.

With out-of-process iframes, it is not safe to assume that an
unresponsive event comes from the main frame's widget/process.

BUG=623228
TEST=See bug for repro steps.

Review-Url: https://codereview.chromium.org/2427423003
Cr-Commit-Position: refs/heads/master@{#468335}
parent 85eea804
......@@ -11,6 +11,9 @@
#include "chrome/grit/generated_resources.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_widget_host.h"
#include "content/public/browser/render_widget_host_view.h"
#include "content/public/common/content_switches.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_utils.h"
......@@ -136,6 +139,65 @@ IN_PROC_BROWSER_TEST_F(SubframeTaskBrowserTest, TaskManagerShowsSubframeTasks) {
simple_page_task->title());
}
// Allows listening to unresponsive task events.
class HungWebContentsTaskManager : public MockWebContentsTaskManager {
public:
HungWebContentsTaskManager() : unresponsive_task_(nullptr) {}
void TaskUnresponsive(Task* task) override { unresponsive_task_ = task; }
Task* unresponsive_task() { return unresponsive_task_; }
private:
Task* unresponsive_task_;
};
// If sites are isolated, makes sure that subframe tasks can react to
// unresponsive renderers.
IN_PROC_BROWSER_TEST_F(SubframeTaskBrowserTest, TaskManagerHungSubframe) {
// This test only makes sense if we have subframe processes.
if (!content::AreAllSitesIsolatedForTesting())
return;
HungWebContentsTaskManager task_manager;
EXPECT_TRUE(task_manager.tasks().empty());
task_manager.StartObserving();
NavigateTo(kCrossSitePageUrl);
// We expect SubframeTasks for b.com and c.com, in either order.
ASSERT_EQ(3U, task_manager.tasks().size());
const Task* subframe_task_1 = task_manager.tasks()[1];
const Task* subframe_task_2 = task_manager.tasks()[2];
EXPECT_EQ(Task::RENDERER, subframe_task_1->GetType());
EXPECT_EQ(Task::RENDERER, subframe_task_2->GetType());
EXPECT_TRUE(base::StartsWith(subframe_task_1->title(),
GetExpectedSubframeTitlePrefix(),
base::CompareCase::INSENSITIVE_ASCII));
EXPECT_TRUE(base::StartsWith(subframe_task_2->title(),
GetExpectedSubframeTitlePrefix(),
base::CompareCase::INSENSITIVE_ASCII));
// Nothing should have hung yet.
EXPECT_EQ(nullptr, task_manager.unresponsive_task());
// Simulate a hang in one of the subframe processes.
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
std::vector<content::RenderFrameHost*> frames = web_contents->GetAllFrames();
content::RenderFrameHost* subframe1 = frames[1];
SimulateUnresponsiveRenderer(web_contents,
subframe1->GetView()->GetRenderWidgetHost());
// Verify task_observer saw one of the two subframe tasks. (There's a race,
// so it could be either one.)
Task* unresponsive_task = task_manager.unresponsive_task();
EXPECT_NE(nullptr, unresponsive_task);
EXPECT_TRUE(unresponsive_task == subframe_task_1 ||
unresponsive_task == subframe_task_2);
}
// A test for top document isolation and how subframes show up in the task
// manager as SubframeTasks.
class SubframeTaskTDIBrowserTest : public InProcessBrowserTest {
......
......@@ -8,6 +8,7 @@
#include "base/macros.h"
#include "chrome/browser/task_manager/providers/web_contents/subframe_task.h"
#include "chrome/browser/task_manager/providers/web_contents/web_contents_tags_manager.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_widget_host.h"
......@@ -161,18 +162,27 @@ void WebContentsEntry::RenderProcessGone(base::TerminationStatus status) {
void WebContentsEntry::OnRendererUnresponsive(
RenderWidgetHost* render_widget_host) {
RendererTask* task = GetTaskForFrame(web_contents()->GetMainFrame());
// Find the first RenderFrameHost matching the RenderWidgetHost.
RendererTask* task = nullptr;
for (const auto& pair : tasks_by_frames_) {
if (pair.first->GetView() == render_widget_host->GetView()) {
DCHECK_EQ(pair.first->GetProcess(), render_widget_host->GetProcess());
task = pair.second;
break;
}
}
if (!task)
return;
DCHECK_EQ(render_widget_host->GetProcess(),
web_contents()->GetMainFrame()->GetProcess());
provider_->NotifyObserverTaskUnresponsive(task);
}
void WebContentsEntry::DidFinishNavigation(
content::NavigationHandle* navigation_handle) {
// We only need to update tasks for main frame navigations.
if (!navigation_handle->IsInMainFrame())
return;
RendererTask* main_frame_task =
GetTaskForFrame(web_contents()->GetMainFrame());
if (!main_frame_task)
......
......@@ -488,6 +488,12 @@ void CrashTab(WebContents* web_contents) {
watcher.Wait();
}
void SimulateUnresponsiveRenderer(WebContents* web_contents,
RenderWidgetHost* widget) {
static_cast<WebContentsImpl*>(web_contents)
->RendererUnresponsive(RenderWidgetHostImpl::From(widget));
}
#if defined(USE_AURA)
bool IsResizeComplete(aura::test::WindowEventDispatcherTestApi* dispatcher_test,
RenderWidgetHostImpl* widget_host) {
......
......@@ -112,6 +112,11 @@ void WaitForResizeComplete(WebContents* web_contents);
// Causes the specified web_contents to crash. Blocks until it is crashed.
void CrashTab(WebContents* web_contents);
// Causes the specified web_contents to issue an OnUnresponsiveRenderer event
// to its observers.
void SimulateUnresponsiveRenderer(WebContents* web_contents,
RenderWidgetHost* widget);
// Simulates clicking at the center of the given tab asynchronously; modifiers
// may contain bits from WebInputEvent::Modifiers.
void SimulateMouseClick(WebContents* web_contents,
......
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