Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(222)

Unified Diff: content/browser/gpu/gpu_ipc_browsertests.cc

Issue 1414683003: Fix gpu command buffer use after free by GrContext (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: blind android fix Created 5 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/gpu/gpu_ipc_browsertests.cc
diff --git a/content/browser/gpu/gpu_ipc_browsertests.cc b/content/browser/gpu/gpu_ipc_browsertests.cc
index 7154c2eea99418188c12363a5cf57a685cc1a1ed..ea842744c6d81c8dec0049b62faa660c0704eadc 100644
--- a/content/browser/gpu/gpu_ipc_browsertests.cc
+++ b/content/browser/gpu/gpu_ipc_browsertests.cc
@@ -13,6 +13,8 @@
#include "content/public/common/content_switches.h"
#include "content/public/test/content_browser_test.h"
#include "gpu/blink/webgraphicscontext3d_in_process_command_buffer_impl.h"
+#include "third_party/skia/include/core/SkSurface.h"
+#include "third_party/skia/include/gpu/GrContext.h"
#include "ui/gl/gl_switches.h"
namespace {
@@ -197,6 +199,59 @@ IN_PROC_BROWSER_TEST_F(BrowserGpuChannelHostFactoryTest,
// Test fails on Chromeos + Mac, flaky on Windows because UI Compositor
// establishes a GPU channel.
#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+#define MAYBE_GrContextKeepsGpuChannelAlive GrContextKeepsGpuChannelAlive
+#else
+#define MAYBE_GrContextKeepsGpuChannelAlive \
+ DISABLED_GrContextKeepsGpuChannelAlive
+#endif
+IN_PROC_BROWSER_TEST_F(BrowserGpuChannelHostFactoryTest,
+ MAYBE_GrContextKeepsGpuChannelAlive) {
+ // Test for crbug.com/551143
+ // This test verifies that holding a reference to the GrContext created by
+ // a ContextProviderCommandBuffer will keep the gpu channel alive after the
+ // provider has been destroyed. Without this behavior, user code would have
+ // to be careful to destroy objects in the right order to avoid using freed
+ // memory as a function pointer in the GrContext's GrGLInterface instance.
+ DCHECK(!IsChannelEstablished());
+ EstablishAndWait();
+
+ GpuChannelHost* host = GetGpuChannel();
+
+ // Expect one ref held by the factory.
+ EXPECT_TRUE(host->HasOneRef());
+
+ // Step 1: sanity check: verify that construction+destruction of context
+ // provider will ref/unref 'host'
+ scoped_refptr<ContextProviderCommandBuffer> provider =
+ ContextProviderCommandBuffer::Create(CreateContext(),
+ OFFSCREEN_CONTEXT_FOR_TESTING);
+ EXPECT_FALSE(host->HasOneRef());
+ provider = nullptr;
+ EXPECT_TRUE(host->HasOneRef());
+
+ // Step 2: verify that holding onto the provider's GrContext will
+ // retain the host after provider is destroyed.
+ provider = ContextProviderCommandBuffer::Create(CreateContext(),
+ OFFSCREEN_CONTEXT_FOR_TESTING);
+ EXPECT_FALSE(host->HasOneRef());
+ skia::RefPtr<GrContext> gr_context = skia::AdoptRef(provider->GrContext());
+ EXPECT_FALSE(host->HasOneRef());
+ provider = nullptr;
+ EXPECT_FALSE(host->HasOneRef());
+
+ SkImageInfo info = SkImageInfo::MakeN32Premul(1, 1);
+ skia::RefPtr<SkSurface> surface = skia::AdoptRef(SkSurface::NewRenderTarget(
+ gr_context.get(), SkSurface::kNo_Budgeted, info));
+ EXPECT_FALSE(host->HasOneRef());
+ gr_context = nullptr;
+ EXPECT_FALSE(host->HasOneRef()); // GrContext kept alive by surface.
+ surface = nullptr;
+ EXPECT_TRUE(host->HasOneRef());
+}
+
+// Test fails on Chromeos + Mac, flaky on Windows because UI Compositor
+// establishes a GPU channel.
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
#define MAYBE_CrashAndRecover
#else
#define MAYBE_CrashAndRecover DISABLED_CrashAndRecover

Powered by Google App Engine
This is Rietveld 408576698