Skip to content

Commit

Permalink
Plugin Power Saver: Fix Dr. Memory leaks in browser tests.
Browse files Browse the repository at this point in the history
Power Saver Test Plugin triggers Dr. Memory leaks under Windows. Most likely because the test process is destroyed before the out of process test plugin has a chance to free its Pepper resources (an image data).

This is because the test plugin is trying to draw new frames as fast as it possibly can. There's no reason for the test plugin to draw so many frames. In fact, it only needs one.

Changing the test plugin to draw only one frame fixes the issue for me (locally).

BUG=487492

Review URL: https://codereview.chromium.org/1182283002

Cr-Commit-Position: refs/heads/master@{#334407}
  • Loading branch information
tommycli authored and Commit bot committed Jun 15, 2015
1 parent f08cb00 commit 0a9a5fe
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 35 deletions.
3 changes: 2 additions & 1 deletion ppapi/examples/gamepad/gamepad.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ class MyInstance : public pp::Instance {
}

void OnFlush(int32_t) {
// This plugin continuously paints because it continously samples the
// gamepad and paints its updated state.
Paint();
}

Expand Down Expand Up @@ -145,4 +147,3 @@ Module* CreateModule() {
}

} // namespace pp

13 changes: 7 additions & 6 deletions ppapi/tests/power_saver_test_plugin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@
#undef PostMessage
#endif

static void DummyCompletionCallback(void*, int32_t) {
}

// This is a simple C++ Pepper plugin that enables Plugin Power Saver tests.
class PowerSaverTestInstance : public pp::Instance {
public:
explicit PowerSaverTestInstance(PP_Instance instance)
: pp::Instance(instance), callback_factory_(this) {}
: pp::Instance(instance) {}
~PowerSaverTestInstance() override {}

bool Init(uint32_t argc, const char* argn[], const char* argv[]) {
Expand All @@ -42,11 +45,11 @@ class PowerSaverTestInstance : public pp::Instance {
if (!BindGraphics(device_context_))
return;

// Since we draw a static image, we only need to make a new frame when
// the device is initialized or the view size changes.
Paint();
}

void OnFlush(int32_t) { Paint(); }

private:
void Paint() {
pp::ImageData image(this, PP_IMAGEDATAFORMAT_BGRA_PREMUL,
Expand All @@ -64,13 +67,11 @@ class PowerSaverTestInstance : public pp::Instance {

device_context_.ReplaceContents(&image);
device_context_.Flush(
callback_factory_.NewCallback(&PowerSaverTestInstance::OnFlush));
pp::CompletionCallback(&DummyCompletionCallback, nullptr));
}

pp::View view_;
pp::Graphics2D device_context_;

pp::CompletionCallbackFactory<PowerSaverTestInstance> callback_factory_;
};

class PowerSaverTestModule : public pp::Module {
Expand Down
28 changes: 0 additions & 28 deletions tools/valgrind/drmemory/suppressions_full.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1872,34 +1872,6 @@ content.dll!content::RenderThreadImpl::AllocateSharedMemory
...
content.dll!content::WebGraphicsContext3DCommandBufferImpl::CreateContext

HANDLE LEAK
name=https://crbug.com/487492
system call NtCreateSection
KERNELBASE.dll!CreateFileMappingW
base.dll!base::SharedMemory::Create
base.dll!base::SharedMemory::CreateAnonymous
content.dll!content::ChildThreadImpl::AllocateSharedMemory
...
ppapi_proxy.dll!ppapi::proxy::PPB_ImageData_Proxy::CreateImageData

HANDLE LEAK
name=https://crbug.com/487492_b
system call NtGdiCreateDIBSection
GDI32.dll!CreateDIBSection
skia.dll!`anonymous namespace'::CreateHBitmap
...
content.dll!content::PepperGraphics2DHost::Flush

HANDLE LEAK
name=https://crbug.com/487492_c
system call NtCreateSection
KERNELBASE.dll!CreateFileMappingW
base.dll!base::SharedMemory::Create
base.dll!base::SharedMemory::CreateAnonymous
content.dll!content::ChildThreadImpl::AllocateSharedMemory
...
content.dll!content::PepperPluginInstanceImpl::PrepareTextureMailbox

HANDLE LEAK
name=https://crbug.com/489779
system call NtUserCreateWindowEx
Expand Down

0 comments on commit 0a9a5fe

Please sign in to comment.