Skip to content

Commit

Permalink
Reland 9034035: Make it possible to have 1 PpapiGlobals per thread.
Browse files Browse the repository at this point in the history
Original CL: r117399, http://codereview.chromium.org/9034035/
Reverted in r117414, http://codereview.chromium.org/9139054/ due to a static initializer.

This is the same as r117399 except using a LazyInstance to eliminate the static initializer.

BUG=
TEST=
TBR=dmichael@chromium.org


Review URL: http://codereview.chromium.org/9187055

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@117475 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
dmichael@chromium.org committed Jan 12, 2012
1 parent a21d808 commit 7309756
Show file tree
Hide file tree
Showing 27 changed files with 351 additions and 223 deletions.
6 changes: 2 additions & 4 deletions ppapi/proxy/host_dispatcher.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 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.

Expand Down Expand Up @@ -71,9 +71,7 @@ HostDispatcher::HostDispatcher(base::ProcessHandle remote_process_handle,
g_module_to_dispatcher = new ModuleToDispatcherMap;
(*g_module_to_dispatcher)[pp_module_] = this;

const PPB_Var* var_interface =
static_cast<const PPB_Var*>(local_get_interface(PPB_VAR_INTERFACE));
SetSerializationRules(new HostVarSerializationRules(var_interface, module));
SetSerializationRules(new HostVarSerializationRules(module));

ppb_proxy_ = reinterpret_cast<const PPB_Proxy_Private*>(
local_get_interface(PPB_PROXY_PRIVATE_INTERFACE));
Expand Down
38 changes: 19 additions & 19 deletions ppapi/proxy/host_var_serialization_rules.cc
Original file line number Diff line number Diff line change
@@ -1,20 +1,24 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 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 "ppapi/proxy/host_var_serialization_rules.h"

#include "base/logging.h"
#include "ppapi/c/ppb_var.h"
#include "ppapi/shared_impl/ppapi_globals.h"
#include "ppapi/shared_impl/var.h"
#include "ppapi/shared_impl/var_tracker.h"

using ppapi::PpapiGlobals;
using ppapi::StringVar;
using ppapi::VarTracker;

namespace ppapi {
namespace proxy {

HostVarSerializationRules::HostVarSerializationRules(
const PPB_Var* var_interface,
PP_Module pp_module)
: var_interface_(var_interface),
pp_module_(pp_module) {
HostVarSerializationRules::HostVarSerializationRules(PP_Module pp_module)
: pp_module_(pp_module) {
}

HostVarSerializationRules::~HostVarSerializationRules() {
Expand All @@ -31,18 +35,15 @@ PP_Var HostVarSerializationRules::BeginReceiveCallerOwned(
const PP_Var& var,
const std::string* str_val,
Dispatcher* /* dispatcher */) {
if (var.type == PP_VARTYPE_STRING) {
// Convert the string to the context of the current process.
return var_interface_->VarFromUtf8(str_val->c_str(),
static_cast<uint32_t>(str_val->size()));
}
if (var.type == PP_VARTYPE_STRING)
return StringVar::StringToPPVar(*str_val);
return var;
}

void HostVarSerializationRules::EndReceiveCallerOwned(const PP_Var& var) {
if (var.type == PP_VARTYPE_STRING) {
// Destroy the string BeginReceiveCallerOwned created above.
var_interface_->Release(var);
PpapiGlobals::Get()->GetVarTracker()->ReleaseVar(var);
}
}

Expand All @@ -51,13 +52,12 @@ PP_Var HostVarSerializationRules::ReceivePassRef(const PP_Var& var,
Dispatcher* /* dispatcher */) {
if (var.type == PP_VARTYPE_STRING) {
// Convert the string to the context of the current process.
return var_interface_->VarFromUtf8(str_val.c_str(),
static_cast<uint32_t>(str_val.size()));
return StringVar::StringToPPVar(str_val);
}

// See PluginVarSerialization::BeginSendPassRef for an example.
if (var.type == PP_VARTYPE_OBJECT)
var_interface_->AddRef(var);
PpapiGlobals::Get()->GetVarTracker()->AddRefVar(var);
return var;
}

Expand All @@ -83,13 +83,13 @@ void HostVarSerializationRules::VarToString(const PP_Var& var,
// This could be optimized to avoid an extra string copy by going to a lower
// level of the browser's implementation of strings where we already have
// a std::string.
uint32_t len = 0;
const char* data = var_interface_->VarToUtf8(var, &len);
str->assign(data, len);
StringVar* string_var = StringVar::FromPPVar(var);
if (string_var)
*str = string_var->value();
}

void HostVarSerializationRules::ReleaseObjectRef(const PP_Var& var) {
var_interface_->Release(var);
PpapiGlobals::Get()->GetVarTracker()->ReleaseVar(var);
}

} // namespace proxy
Expand Down
4 changes: 1 addition & 3 deletions ppapi/proxy/host_var_serialization_rules.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ namespace proxy {
// Implementation of the VarSerializationRules interface for the host side.
class HostVarSerializationRules : public VarSerializationRules {
public:
HostVarSerializationRules(const PPB_Var* var_interface,
PP_Module pp_module);
HostVarSerializationRules(PP_Module pp_module);
~HostVarSerializationRules();

// VarSerialization implementation.
Expand All @@ -40,7 +39,6 @@ class HostVarSerializationRules : public VarSerializationRules {
// string object.
void VarToString(const PP_Var& var, std::string* str);

const PPB_Var* var_interface_;
PP_Module pp_module_;

DISALLOW_COPY_AND_ASSIGN(HostVarSerializationRules);
Expand Down
23 changes: 21 additions & 2 deletions ppapi/proxy/plugin_globals.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 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.

Expand All @@ -19,8 +19,15 @@ PluginGlobals::PluginGlobals()
plugin_globals_ = this;
}

PluginGlobals::PluginGlobals(ForTest for_test)
: ppapi::PpapiGlobals(for_test),
plugin_proxy_delegate_(NULL),
callback_tracker_(new CallbackTracker) {
DCHECK(!plugin_globals_);
}

PluginGlobals::~PluginGlobals() {
DCHECK(plugin_globals_ == this);
DCHECK(plugin_globals_ == this || !plugin_globals_);
plugin_globals_ = NULL;
}

Expand Down Expand Up @@ -51,5 +58,17 @@ PP_Module PluginGlobals::GetModuleForInstance(PP_Instance instance) {
return 0;
}

base::Lock* PluginGlobals::GetProxyLock() {
#ifdef ENABLE_PEPPER_THREADING
return &proxy_lock_;
#else
return NULL;
#endif
}

bool PluginGlobals::IsPluginGlobals() const {
return true;
}

} // namespace proxy
} // namespace ppapi
14 changes: 12 additions & 2 deletions ppapi/proxy/plugin_globals.h
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 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.

#ifndef PPAPI_PROXY_PLUGIN_GLOBALS_H_
#define PPAPI_PROXY_PLUGIN_GLOBALS_H_

#include "base/compiler_specific.h"
#include "base/synchronization/lock.h"
#include "ppapi/proxy/plugin_resource_tracker.h"
#include "ppapi/proxy/plugin_var_tracker.h"
#include "ppapi/proxy/ppapi_proxy_export.h"
Expand All @@ -20,12 +21,16 @@ class PluginProxyDelegate;
class PPAPI_PROXY_EXPORT PluginGlobals : public PpapiGlobals {
public:
PluginGlobals();
PluginGlobals(PpapiGlobals::ForTest);
virtual ~PluginGlobals();

// Getter for the global singleton. Generally, you should use
// PpapiGlobals::Get() when possible. Use this only when you need some
// plugin-specific functionality.
inline static PluginGlobals* Get() { return plugin_globals_; }
inline static PluginGlobals* Get() {
DCHECK(PpapiGlobals::Get()->IsPluginGlobals());
return static_cast<PluginGlobals*>(PpapiGlobals::Get());
}

// PpapiGlobals implementation.
virtual ResourceTracker* GetResourceTracker() OVERRIDE;
Expand All @@ -35,6 +40,7 @@ class PPAPI_PROXY_EXPORT PluginGlobals : public PpapiGlobals {
virtual FunctionGroupBase* GetFunctionAPI(PP_Instance inst,
ApiID id) OVERRIDE;
virtual PP_Module GetModuleForInstance(PP_Instance instance) OVERRIDE;
virtual base::Lock* GetProxyLock() OVERRIDE;

// Getters for the plugin-specific versions.
PluginResourceTracker* plugin_resource_tracker() {
Expand All @@ -53,12 +59,16 @@ class PPAPI_PROXY_EXPORT PluginGlobals : public PpapiGlobals {
}

private:
// PpapiGlobals overrides.
virtual bool IsPluginGlobals() const OVERRIDE;

static PluginGlobals* plugin_globals_;

PluginProxyDelegate* plugin_proxy_delegate_;
PluginResourceTracker plugin_resource_tracker_;
PluginVarTracker plugin_var_tracker_;
scoped_refptr<CallbackTracker> callback_tracker_;
base::Lock proxy_lock_;

DISALLOW_COPY_AND_ASSIGN(PluginGlobals);
};
Expand Down
10 changes: 1 addition & 9 deletions ppapi/proxy/plugin_resource_tracker.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 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.

Expand All @@ -18,17 +18,9 @@ namespace ppapi {
namespace proxy {

PluginResourceTracker::PluginResourceTracker() {
#ifdef ENABLE_PEPPER_THREADING
// Set the global proxy lock, since the plugin-side of the proxy needs to be
// synchronized.
ppapi::ProxyLock::Set(&proxy_lock_);
#endif
}

PluginResourceTracker::~PluginResourceTracker() {
#ifdef ENABLE_PEPPER_THREADING
ppapi::ProxyLock::Reset();
#endif
}

PP_Resource PluginResourceTracker::PluginResourceForHostResource(
Expand Down
6 changes: 1 addition & 5 deletions ppapi/proxy/plugin_resource_tracker.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 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.

Expand All @@ -9,7 +9,6 @@
#include <utility>

#include "base/compiler_specific.h"
#include "base/synchronization/lock.h"
#include "ppapi/c/pp_completion_callback.h"
#include "ppapi/c/pp_instance.h"
#include "ppapi/c/pp_stdint.h"
Expand Down Expand Up @@ -45,9 +44,6 @@ class PPAPI_PROXY_EXPORT PluginResourceTracker : public ResourceTracker {
typedef std::map<HostResource, PP_Resource> HostResourceMap;
HostResourceMap host_resource_map_;

// The global lock for the plugin side of the proxy.
base::Lock proxy_lock_;

DISALLOW_COPY_AND_ASSIGN(PluginResourceTracker);
};

Expand Down
4 changes: 2 additions & 2 deletions ppapi/proxy/plugin_var_serialization_rules.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 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.

Expand Down Expand Up @@ -85,7 +85,7 @@ PP_Var PluginVarSerializationRules::ReceivePassRef(const PP_Var& var,
// plugin code started to return a value, which means it gets another ref
// on behalf of the caller. This needs to be transferred to the plugin and
// folded in to its set of refs it maintains (with one ref representing all
// fo them in the browser).
// of them in the browser).
if (var.type == PP_VARTYPE_OBJECT) {
DCHECK(dispatcher->IsPlugin());
return var_tracker_->ReceiveObjectPassRef(
Expand Down
17 changes: 1 addition & 16 deletions ppapi/proxy/plugin_var_tracker.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 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.

Expand Down Expand Up @@ -141,21 +141,6 @@ void PluginVarTracker::ReleaseHostObject(PluginDispatcher* dispatcher,
ReleaseVar(found->second);
}

int PluginVarTracker::GetRefCountForObject(const PP_Var& plugin_object) {
VarMap::iterator found = GetLiveVar(plugin_object);
if (found == live_vars_.end())
return -1;
return found->second.ref_count;
}

int PluginVarTracker::GetTrackedWithNoReferenceCountForObject(
const PP_Var& plugin_object) {
VarMap::iterator found = GetLiveVar(plugin_object);
if (found == live_vars_.end())
return -1;
return found->second.track_with_no_reference_count;
}

ArrayBufferVar* PluginVarTracker::CreateArrayBuffer(uint32 size_in_bytes) {
return new PluginArrayBufferVar(size_in_bytes);
}
Expand Down
8 changes: 1 addition & 7 deletions ppapi/proxy/plugin_var_tracker.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 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.

Expand Down Expand Up @@ -56,12 +56,6 @@ class PPAPI_PROXY_EXPORT PluginVarTracker : public VarTracker {
void ReleaseHostObject(PluginDispatcher* dispatcher,
const PP_Var& host_object);

// Retrieves the internal reference counts for testing. Returns 0 if we
// know about the object but the corresponding value is 0, or -1 if the
// given object ID isn't in our map.
int GetRefCountForObject(const PP_Var& plugin_object);
int GetTrackedWithNoReferenceCountForObject(const PP_Var& plugin_object);

private:
// VarTracker protected overrides.
virtual int32 AddVarInternal(Var* var, AddVarRefMode mode) OVERRIDE;
Expand Down
16 changes: 12 additions & 4 deletions ppapi/proxy/ppapi_proxy_test.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 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.

Expand Down Expand Up @@ -139,7 +139,8 @@ bool ProxyTestHarnessBase::SupportsInterface(const char* name) {

// PluginProxyTestHarness ------------------------------------------------------

PluginProxyTestHarness::PluginProxyTestHarness() {
PluginProxyTestHarness::PluginProxyTestHarness()
: plugin_globals_(PpapiGlobals::ForTest()) {
}

PluginProxyTestHarness::~PluginProxyTestHarness() {
Expand All @@ -151,6 +152,7 @@ Dispatcher* PluginProxyTestHarness::GetDispatcher() {

void PluginProxyTestHarness::SetUpHarness() {
// These must be first since the dispatcher set-up uses them.
PpapiGlobals::SetPpapiGlobalsOnThreadForTest(GetGlobals());
resource_tracker().DidCreateInstance(pp_instance());

plugin_dispatcher_.reset(new PluginDispatcher(
Expand All @@ -166,6 +168,7 @@ void PluginProxyTestHarness::SetUpHarnessWithChannel(
base::WaitableEvent* shutdown_event,
bool is_client) {
// These must be first since the dispatcher set-up uses them.
PpapiGlobals::SetPpapiGlobalsOnThreadForTest(GetGlobals());
resource_tracker().DidCreateInstance(pp_instance());
plugin_delegate_mock_.Init(ipc_message_loop, shutdown_event);

Expand Down Expand Up @@ -248,7 +251,8 @@ void PluginProxyTest::TearDown() {

// HostProxyTestHarness --------------------------------------------------------

HostProxyTestHarness::HostProxyTestHarness() {
HostProxyTestHarness::HostProxyTestHarness()
: host_globals_(PpapiGlobals::ForTest()) {
}

HostProxyTestHarness::~HostProxyTestHarness() {
Expand All @@ -259,6 +263,8 @@ Dispatcher* HostProxyTestHarness::GetDispatcher() {
}

void HostProxyTestHarness::SetUpHarness() {
// These must be first since the dispatcher set-up uses them.
PpapiGlobals::SetPpapiGlobalsOnThreadForTest(GetGlobals());
host_dispatcher_.reset(new HostDispatcher(
base::Process::Current().handle(),
pp_module(),
Expand All @@ -272,7 +278,10 @@ void HostProxyTestHarness::SetUpHarnessWithChannel(
base::MessageLoopProxy* ipc_message_loop,
base::WaitableEvent* shutdown_event,
bool is_client) {
// These must be first since the dispatcher set-up uses them.
PpapiGlobals::SetPpapiGlobalsOnThreadForTest(GetGlobals());
delegate_mock_.Init(ipc_message_loop, shutdown_event);

host_dispatcher_.reset(new HostDispatcher(
base::Process::Current().handle(),
pp_module(),
Expand Down Expand Up @@ -345,7 +354,6 @@ void TwoWayTest::SetUp() {

IPC::ChannelHandle handle;
handle.name = "TwoWayTestChannel";

base::WaitableEvent remote_harness_set_up(true, false);
plugin_thread_.message_loop_proxy()->PostTask(
FROM_HERE,
Expand Down
Loading

0 comments on commit 7309756

Please sign in to comment.