Commit e052cda2 authored by ckitagawa's avatar ckitagawa Committed by Commit Bot

[Paint Preview] Attempt to fix crash

This CL attempts to fix a crash that was happening when attempting to
use a render frame host that might have been deleted or was in the
process of being deleted.

Bug: 1140163
Change-Id: I9abeaad7c84fa0409c5c152e69610da1db4b44c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485585
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: default avatarMehran Mahmoudi <mahmoudi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819147}
parent b5c5cc3a
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/task/task_traits.h" #include "base/task/task_traits.h"
#include "components/paint_preview/browser/file_manager.h" #include "components/paint_preview/browser/file_manager.h"
#include "components/paint_preview/browser/warm_compositor.h" #include "components/paint_preview/browser/warm_compositor.h"
#include "content/public/browser/render_process_host.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
...@@ -109,6 +110,7 @@ void PaintPreviewTabService::CaptureTab(int tab_id, ...@@ -109,6 +110,7 @@ void PaintPreviewTabService::CaptureTab(int tab_id,
base::BindOnce(&PaintPreviewTabService::CaptureTabInternal, base::BindOnce(&PaintPreviewTabService::CaptureTabInternal,
weak_ptr_factory_.GetWeakPtr(), tab_id, key, weak_ptr_factory_.GetWeakPtr(), tab_id, key,
contents->GetMainFrame()->GetFrameTreeNodeId(), contents->GetMainFrame()->GetFrameTreeNodeId(),
contents->GetMainFrame()->GetGlobalFrameRoutingId(),
std::move(callback))); std::move(callback)));
} }
...@@ -228,6 +230,7 @@ void PaintPreviewTabService::CaptureTabInternal( ...@@ -228,6 +230,7 @@ void PaintPreviewTabService::CaptureTabInternal(
int tab_id, int tab_id,
const DirectoryKey& key, const DirectoryKey& key,
int frame_tree_node_id, int frame_tree_node_id,
content::GlobalFrameRoutingId frame_routing_id,
FinishedCallback callback, FinishedCallback callback,
const base::Optional<base::FilePath>& file_path) { const base::Optional<base::FilePath>& file_path) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
...@@ -237,7 +240,9 @@ void PaintPreviewTabService::CaptureTabInternal( ...@@ -237,7 +240,9 @@ void PaintPreviewTabService::CaptureTabInternal(
} }
auto* contents = auto* contents =
content::WebContents::FromFrameTreeNodeId(frame_tree_node_id); content::WebContents::FromFrameTreeNodeId(frame_tree_node_id);
if (!contents) { auto* rfh = content::RenderFrameHost::FromID(frame_routing_id);
if (!contents || !rfh || contents->IsBeingDestroyed() ||
contents->GetMainFrame() != rfh || !rfh->IsCurrent()) {
std::move(callback).Run(Status::kWebContentsGone); std::move(callback).Run(Status::kWebContentsGone);
return; return;
} }
......
...@@ -113,6 +113,7 @@ class PaintPreviewTabService : public PaintPreviewBaseService { ...@@ -113,6 +113,7 @@ class PaintPreviewTabService : public PaintPreviewBaseService {
void CaptureTabInternal(int tab_id, void CaptureTabInternal(int tab_id,
const DirectoryKey& key, const DirectoryKey& key,
int frame_tree_node_id, int frame_tree_node_id,
content::GlobalFrameRoutingId frame_routing_id,
FinishedCallback callback, FinishedCallback callback,
const base::Optional<base::FilePath>& file_path); const base::Optional<base::FilePath>& file_path);
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/test/task_environment.h" #include "base/test/task_environment.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h" #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/test/navigation_simulator.h" #include "content/public/test/navigation_simulator.h"
#include "content/public/test/test_utils.h" #include "content/public/test/test_utils.h"
#include "mojo/public/cpp/bindings/associated_receiver.h" #include "mojo/public/cpp/bindings/associated_receiver.h"
...@@ -83,6 +84,9 @@ class PaintPreviewTabServiceTest : public ChromeRenderViewHostTestHarness { ...@@ -83,6 +84,9 @@ class PaintPreviewTabServiceTest : public ChromeRenderViewHostTestHarness {
protected: protected:
void SetUp() override { void SetUp() override {
ChromeRenderViewHostTestHarness::SetUp(); ChromeRenderViewHostTestHarness::SetUp();
NavigateAndCommit(GURL("https://www.example.com/"),
ui::PageTransition::PAGE_TRANSITION_FIRST);
task_environment()->RunUntilIdle();
EXPECT_TRUE(temp_dir_.CreateUniqueTempDir()); EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
service_ = std::make_unique<PaintPreviewTabService>( service_ = std::make_unique<PaintPreviewTabService>(
temp_dir_.GetPath(), kFeatureName, nullptr, false); temp_dir_.GetPath(), kFeatureName, nullptr, false);
...@@ -90,6 +94,8 @@ class PaintPreviewTabServiceTest : public ChromeRenderViewHostTestHarness { ...@@ -90,6 +94,8 @@ class PaintPreviewTabServiceTest : public ChromeRenderViewHostTestHarness {
EXPECT_TRUE(service_->CacheInitialized()); EXPECT_TRUE(service_->CacheInitialized());
} }
void TearDown() override { ChromeRenderViewHostTestHarness::TearDown(); }
PaintPreviewTabService* GetService() { return service_.get(); } PaintPreviewTabService* GetService() { return service_.get(); }
void OverrideInterface(MockPaintPreviewRecorder* recorder) { void OverrideInterface(MockPaintPreviewRecorder* recorder) {
...@@ -128,8 +134,6 @@ class PaintPreviewTabServiceTest : public ChromeRenderViewHostTestHarness { ...@@ -128,8 +134,6 @@ class PaintPreviewTabServiceTest : public ChromeRenderViewHostTestHarness {
}; };
TEST_F(PaintPreviewTabServiceTest, CaptureTab) { TEST_F(PaintPreviewTabServiceTest, CaptureTab) {
content::NavigationSimulator::NavigateAndCommitFromBrowser(
web_contents(), GURL("http://www.example.com"));
const int kTabId = 1U; const int kTabId = 1U;
MockPaintPreviewRecorder recorder; MockPaintPreviewRecorder recorder;
...@@ -163,8 +167,6 @@ TEST_F(PaintPreviewTabServiceTest, CaptureTab) { ...@@ -163,8 +167,6 @@ TEST_F(PaintPreviewTabServiceTest, CaptureTab) {
} }
TEST_F(PaintPreviewTabServiceTest, CaptureTabFailed) { TEST_F(PaintPreviewTabServiceTest, CaptureTabFailed) {
content::NavigationSimulator::NavigateAndCommitFromBrowser(
web_contents(), GURL("http://www.example.com"));
const int kTabId = 1U; const int kTabId = 1U;
MockPaintPreviewRecorder recorder; MockPaintPreviewRecorder recorder;
...@@ -199,8 +201,6 @@ TEST_F(PaintPreviewTabServiceTest, CaptureTabFailed) { ...@@ -199,8 +201,6 @@ TEST_F(PaintPreviewTabServiceTest, CaptureTabFailed) {
} }
TEST_F(PaintPreviewTabServiceTest, CaptureTabTwice) { TEST_F(PaintPreviewTabServiceTest, CaptureTabTwice) {
content::NavigationSimulator::NavigateAndCommitFromBrowser(
web_contents(), GURL("http://www.example.com"));
const int kTabId = 1U; const int kTabId = 1U;
MockPaintPreviewRecorder recorder; MockPaintPreviewRecorder recorder;
...@@ -387,8 +387,6 @@ TEST_F(PaintPreviewTabServiceTest, EarlyAudit) { ...@@ -387,8 +387,6 @@ TEST_F(PaintPreviewTabServiceTest, EarlyAudit) {
} }
TEST_F(PaintPreviewTabServiceTest, EarlyCapture) { TEST_F(PaintPreviewTabServiceTest, EarlyCapture) {
content::NavigationSimulator::NavigateAndCommitFromBrowser(
web_contents(), GURL("http://www.example.com"));
const int kTabId = 1U; const int kTabId = 1U;
MockPaintPreviewRecorder recorder; MockPaintPreviewRecorder recorder;
...@@ -423,8 +421,6 @@ TEST_F(PaintPreviewTabServiceTest, EarlyCapture) { ...@@ -423,8 +421,6 @@ TEST_F(PaintPreviewTabServiceTest, EarlyCapture) {
} }
TEST_F(PaintPreviewTabServiceTest, CaptureTabAndCleanup) { TEST_F(PaintPreviewTabServiceTest, CaptureTabAndCleanup) {
content::NavigationSimulator::NavigateAndCommitFromBrowser(
web_contents(), GURL("http://www.example.com"));
const int kTabId = 1U; const int kTabId = 1U;
MockPaintPreviewRecorder recorder; MockPaintPreviewRecorder recorder;
......
...@@ -275,6 +275,7 @@ void PaintPreviewClient::CapturePaintPreview( ...@@ -275,6 +275,7 @@ void PaintPreviewClient::CapturePaintPreview(
const PaintPreviewParams& params, const PaintPreviewParams& params,
content::RenderFrameHost* render_frame_host, content::RenderFrameHost* render_frame_host,
PaintPreviewCallback callback) { PaintPreviewCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (base::Contains(all_document_data_, params.inner.document_guid)) { if (base::Contains(all_document_data_, params.inner.document_guid)) {
std::move(callback).Run(params.inner.document_guid, std::move(callback).Run(params.inner.document_guid,
mojom::PaintPreviewStatus::kGuidCollision, {}); mojom::PaintPreviewStatus::kGuidCollision, {});
......
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