PPAPI: Make V8VarConverterTest use only 1 thread

PPAPI: Make V8VarConverterTest use only 1 thread

This should be a little faster, since we don't need to start a second thread.
More importantly, it better reflects what V8VarConverter does in production code.

BUG=
R=raymes@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=223351

[raymes, 2013-09-13 20:24:12]

lgtm

Thanks! I was trying to figure out how to do this in the first place.

On Fri, Sep 13, 2013 at 12:40 PM,  <dmichael@chromium.org> wrote:
> Reviewers: raymes, teravest,
>
> Description:
> PPAPI: Make V8VarConverterTest use only 1 thread
>
> This should be a little faster, since we don't need to start a second
> thread.
> More importantly, it better reflects what V8VarConverter does in production
> code.
>
> BUG=
>
> Please review this at https://codereview.chromium.org/23522043/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+12, -16 lines):
>   M content/renderer/pepper/v8_var_converter.h
>   M content/renderer/pepper/v8_var_converter.cc
>   M content/renderer/pepper/v8_var_converter_unittest.cc
>
>
> Index: content/renderer/pepper/v8_var_converter.cc
> diff --git a/content/renderer/pepper/v8_var_converter.cc
> b/content/renderer/pepper/v8_var_converter.cc
> index
>
e11aa6d2fdfde0941742f2e48c1f640597c8b439..062922aed126a7ed5b61df94ec7eaeb4fccddd42
> 100644
> --- a/content/renderer/pepper/v8_var_converter.cc
> +++ b/content/renderer/pepper/v8_var_converter.cc
> @@ -246,9 +246,8 @@ V8VarConverter::V8VarConverter(PP_Instance instance)
>
>  V8VarConverter::V8VarConverter(
>      PP_Instance instance,
> -    const scoped_refptr<base::MessageLoopProxy>& message_loop_proxy,
>      scoped_ptr<ResourceConverter> resource_converter)
> -    : message_loop_proxy_(message_loop_proxy),
> +    : message_loop_proxy_(base::MessageLoopProxy::current()),
>        resource_converter_(resource_converter.release()) {
>  }
>
> Index: content/renderer/pepper/v8_var_converter.h
> diff --git a/content/renderer/pepper/v8_var_converter.h
> b/content/renderer/pepper/v8_var_converter.h
> index
>
2b879fa53a68c9e7897faa8010bd8f91360d0345..37ce20824d0b0707b163ad1be3a9ac44a9963f0b
> 100644
> --- a/content/renderer/pepper/v8_var_converter.h
> +++ b/content/renderer/pepper/v8_var_converter.h
> @@ -29,7 +29,6 @@ class CONTENT_EXPORT V8VarConverter {
>    // Constructor for testing.
>    V8VarConverter(
>        PP_Instance instance,
> -      const scoped_refptr<base::MessageLoopProxy>& message_loop_proxy,
>        scoped_ptr<ResourceConverter> resource_converter);
>    ~V8VarConverter();
>
> Index: content/renderer/pepper/v8_var_converter_unittest.cc
> diff --git a/content/renderer/pepper/v8_var_converter_unittest.cc
> b/content/renderer/pepper/v8_var_converter_unittest.cc
> index
>
095d70c39d48d0008aa0a9b8c4724d6338b19cbb..195210afed8eaef4b677df00a97a9ed68ce2dacd
> 100644
> --- a/content/renderer/pepper/v8_var_converter_unittest.cc
> +++ b/content/renderer/pepper/v8_var_converter_unittest.cc
> @@ -9,8 +9,9 @@
>  #include "base/logging.h"
>  #include "base/memory/ref_counted.h"
>  #include "base/memory/scoped_ptr.h"
> +#include "base/message_loop/message_loop.h"
> +#include "base/run_loop.h"
>  #include "base/synchronization/waitable_event.h"
> -#include "base/threading/thread.h"
>  #include "base/values.h"
>  #include "content/renderer/pepper/resource_converter.h"
>  #include "ppapi/c/pp_bool.h"
> @@ -145,14 +146,10 @@ class V8VarConverterTest : public testing::Test {
>   public:
>    V8VarConverterTest()
>        : isolate_(v8::Isolate::GetCurrent()),
> -        conversion_success_(false),
> -        conversion_event_(true, false),
> -        callback_thread_("callback_thread") {
> -    callback_thread_.Start();
> +        conversion_success_(false) {
>      PP_Instance dummy = 1234;
>      converter_.reset(new V8VarConverter(
>          dummy,
> -        callback_thread_.message_loop_proxy(),
>          scoped_ptr<ResourceConverter>(new MockResourceConverter).Pass()));
>    }
>    virtual ~V8VarConverterTest() {}
> @@ -174,22 +171,24 @@ class V8VarConverterTest : public testing::Test {
>    bool FromV8ValueSync(v8::Handle<v8::Value> val,
>                         v8::Handle<v8::Context> context,
>                         PP_Var* result) {
> +    base::RunLoop loop;
>      converter_->FromV8Value(val, context, base::Bind(
> -        &V8VarConverterTest::FromV8ValueComplete, base::Unretained(this)));
> -    conversion_event_.Wait();
> -    conversion_event_.Reset();
> +        &V8VarConverterTest::FromV8ValueComplete, base::Unretained(this),
> +        loop.QuitClosure()));
>      if (conversion_success_)
>        *result = conversion_result_;
>      return conversion_success_;
>    }
>
> -  void FromV8ValueComplete(const ScopedPPVar& scoped_var, bool success) {
> +  void FromV8ValueComplete(base::Closure quit_closure,
> +                           const ScopedPPVar& scoped_var,
> +                           bool success) {
>      conversion_success_ = success;
>      if (success) {
>        ScopedPPVar var = scoped_var;
>        conversion_result_ = var.Release();
>      }
> -    conversion_event_.Signal();
> +    quit_closure.Run();
>    }
>
>    bool RoundTrip(const PP_Var& var, PP_Var* result) {
> @@ -229,8 +228,7 @@ class V8VarConverterTest : public testing::Test {
>
>    PP_Var conversion_result_;
>    bool conversion_success_;
> -  base::WaitableEvent conversion_event_;
> -  base::Thread callback_thread_;
> +  base::MessageLoop message_loop_;
>  };
>
>  }  // namespace
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2013-09-13 20:35:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmichael@chromium.org/23522043/4001

[commit-bot: I haz the power, 2013-09-14 04:21:28]

Retried try job too often on win_rel for step(s) chrome_frame_net_tests,
chrome_frame_tests, chrome_frame_unittests, nacl_integration,
sync_integration_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[dmichael (off chromium), 2013-09-16 17:13:33]

Committed patchset #3 manually as r223351 (presubmit successful).