Pepper API implementation for output protection.

chrome/browser/renderer_host/pepper/pepper_output_protection_host.cc

Index: chrome/browser/renderer_host/pepper/pepper_output_protection_host.cc
diff --git a/chrome/browser/renderer_host/pepper/pepper_output_protection_host.cc b/chrome/browser/renderer_host/pepper/pepper_output_protection_host.cc
new file mode 100644
index 0000000000000000000000000000000000000000..878a915a8ad96fa4e6db92117b3bc4791f297e4b
--- /dev/null
+++ b/chrome/browser/renderer_host/pepper/pepper_output_protection_host.cc
@@ -0,0 +1,129 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/renderer_host/pepper/pepper_output_protection_host.h"
+
+#include "build/build_config.h"
+#include "content/public/browser/browser_ppapi_host.h"
+#include "content/public/browser/browser_thread.h"
+#include "ipc/ipc_message.h"
+#include "ppapi/c/pp_errors.h"
+#include "ppapi/c/private/ppb_output_protection_private.h"
+#include "ppapi/host/dispatch_host_message.h"
+#include "ppapi/host/host_message_context.h"
+#include "ppapi/host/ppapi_host.h"
+#include "ppapi/proxy/ppapi_messages.h"
+
+#if defined(USE_ASH)
+#include "ash/shell.h"
+#include "ash/shell_delegate.h"
+#include "chromeos/display/output_configurator.h"
+#endif
+
+namespace chrome {
+
+PepperOutputProtectionHost::PepperOutputProtectionHost(
+    content::BrowserPpapiHost* host,
+    PP_Instance instance,
+    PP_Resource resource)
+    : ResourceHost(host->GetPpapiHost(), instance, resource),
+      renderer_ppapi_host_(host) {
+}
+
+PepperOutputProtectionHost::~PepperOutputProtectionHost() {
+#if defined(OS_CHROMEOS) && defined(USE_ASH) && defined(USE_X11)

[ddorwin, 2013/09/12 04:22:13]

What is the purpose of ASH and X11? Is there a better value we could define? At
least in this file since it appears multiple times.

[kcwu, 2013/09/12 18:22:08]

USE_X11: Output protection implementation requires xrandr
USE_ASH: output_configurator() is only available if defined(USE_ASH)

+  chromeos::OutputConfigurator* configurator =

[DaleCurtis, 2013/09/12 01:23:13]

Is it safe to call this from any thread?  I don't see any DCHECKs in
ash::Shell::GetInstance(), but I'm not familiar with that code.

[kcwu, 2013/09/16 11:57:37]

After check the code, I feel they assume all run in UI thread. So I added two
ipc messages to make my init and termination code running on UI thread as well.

[DaleCurtis, 2013/09/16 18:07:08]

I don't understand why you added those two new IPCs? Why not just post a task to
the UI thread during construction like you were doing during destruction?

+      ash::Shell::GetInstance()->output_configurator();
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(base::IgnoreResult(
+                     &chromeos::OutputConfigurator::EnableOutputProtection),
+                 base::Unretained(configurator),
+                 base::Unretained(this),

[DaleCurtis, 2013/09/12 01:23:13]

A comment indicating that this is only used as a key and not dereferenced should
be added.  Otherwise this looks like a potential for use-after-free.

[ddorwin, 2013/09/12 04:22:13]

Passing |this| in the destructor seems dangerous. Can we pass NULL if we assume
this will be gone?

[kcwu, 2013/09/12 18:22:08]

No. That will be a small memory leak.

[kcwu, 2013/09/12 18:22:08]

Done.

+                 PP_OUTPUT_PROTECTION_METHOD_PRIVATE_NONE));
+#else
+  NOTIMPLEMENTED();

[ddorwin, 2013/09/12 04:22:13]

Wouldn't it be better to indicate this when the object is created too?

[kcwu, 2013/09/12 18:22:08]

Done.

+#endif
+}
+
+int32_t PepperOutputProtectionHost::OnResourceMessageReceived(
+    const IPC::Message& msg,
+    ppapi::host::HostMessageContext* context) {
+  IPC_BEGIN_MESSAGE_MAP(PepperOutputProtectionHost, msg)
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL_0(
+        PpapiHostMsg_OutputProtection_QueryStatus,
+        OnQueryStatus);
+    PPAPI_DISPATCH_HOST_RESOURCE_CALL(
+        PpapiHostMsg_OutputProtection_EnableProtection,
+        OnEnableProtection);
+  IPC_END_MESSAGE_MAP()
+  return PP_ERROR_FAILED;
+}
+
+void PepperOutputProtectionHost::QueryStatusOnUIThread(
+    ppapi::host::ReplyMessageContext reply_context) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+#if defined(OS_CHROMEOS) && defined(USE_ASH) && defined(USE_X11)
+  chromeos::OutputConfigurator* configurator =
+      ash::Shell::GetInstance()->output_configurator();
+  uint32_t link_mask = 0, protection_mask = 0;

[ddorwin, 2013/09/12 04:22:13]

These need to be defined outside the #if block or you'll get a compile error at
82.

Should these be defined on separate lines?

[kcwu, 2013/09/12 18:22:08]

Done.

+  bool result = configurator->QueryOutputProtectionStatus(
+      this, &link_mask, &protection_mask);
+  reply_context.params.set_result(result ? PP_OK : PP_ERROR_FAILED);
+#else
+  NOTIMPLEMENTED();
+  reply_context.params.set_result(PP_ERROR_NOTSUPPORTED);
+#endif
+  host()->SendReply(reply_context,

[DaleCurtis, 2013/09/12 01:23:13]

invalid wrapping.  reply_context must go on the next line per style guide.

[kcwu, 2013/09/12 18:22:08]

Done.

+      PpapiPluginMsg_OutputProtection_QueryStatusReply(
+          link_mask, protection_mask));
+}
+
+int32_t PepperOutputProtectionHost::OnQueryStatus(
+    ppapi::host::HostMessageContext* context) {
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(&PepperOutputProtectionHost::QueryStatusOnUIThread,
+                 base::Unretained(this),
+                 context->MakeReplyMessageContext()));
+  return PP_OK_COMPLETIONPENDING;
+}
+
+void PepperOutputProtectionHost::EnableProtectionOnUIThread(
+    ppapi::host::ReplyMessageContext reply_context,
+    uint32_t desired_method_mask) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+#if defined(OS_CHROMEOS) && defined(USE_ASH) && defined(USE_X11)
+  chromeos::OutputConfigurator* configurator =
+      ash::Shell::GetInstance()->output_configurator();
+  bool result = configurator->EnableOutputProtection(
+      this, desired_method_mask);
+  reply_context.params.set_result(result ? PP_OK : PP_ERROR_FAILED);
+#else
+  NOTIMPLEMENTED();
+  reply_context.params.set_result(PP_ERROR_NOTSUPPORTED);
+#endif
+  host()->SendReply(reply_context,

[DaleCurtis, 2013/09/12 01:23:13]

Wrapping.

[kcwu, 2013/09/12 18:22:08]

Done.

+      PpapiPluginMsg_OutputProtection_EnableProtectionReply());
+}
+
+int32_t PepperOutputProtectionHost::OnEnableProtection(
+    ppapi::host::HostMessageContext* context,
+    uint32_t desired_method_mask) {
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(&PepperOutputProtectionHost::EnableProtectionOnUIThread,
+                 base::Unretained(this),
+                 context->MakeReplyMessageContext(),
+                 desired_method_mask));
+  return PP_OK_COMPLETIONPENDING;
+}
+
+}  // namespace chrome
+