Chromium Code Reviews
Help | Chromium Project | Sign in
(42)

Issue 2856067: Revert 53892 - Initial scriptable object implementation.... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 10 months ago by bryeung
Modified:
4 years ago
Reviewers:
awong (On leave), kobi
CC:
chromium-reviews, Alpha Left Google, Sergey Ulanov, dmac, awong (On leave), garykac, Paweł Hajdan Jr.
Visibility:
Public.

Description

Revert 53892 - Initial scriptable object implementation. Broke ARM build, reverting. Errors from log: remoting/client/plugin/chromoting_scriptable_object.cc: In member function 'virtual bool remoting::ChromotingScriptableObject::HasProperty(const pp::Var&, pp::Var*)': remoting/client/plugin/chromoting_scriptable_object.cc:48: error: NULL used in arithmetic remoting/client/plugin/chromoting_scriptable_object.cc: In member function 'virtual bool remoting::ChromotingScriptableObject::HasMethod(const pp::Var&, pp::Var*)': remoting/client/plugin/chromoting_scriptable_object.cc:63: error: NULL used in arithmetic BUG=50248 TEST=write javascript to manually setup connection. Review URL: http://codereview.chromium.org/3064009 TBR=ajwong@chromium.org

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+107 lines, -329 lines) Patch
M remoting/client/client_util.h View 1 chunk +4 lines, -0 lines 0 comments Download
M remoting/client/client_util.cc View 1 chunk +38 lines, -0 lines 0 comments Download
M remoting/client/client_util_unittest.cc View 1 chunk +16 lines, -1 line 0 comments Download
M remoting/client/plugin/chromoting_plugin.h View 4 chunks +7 lines, -7 lines 0 comments Download
M remoting/client/plugin/chromoting_plugin.cc View 4 chunks +38 lines, -39 lines 0 comments Download
D remoting/client/plugin/chromoting_scriptable_object.h View 1 chunk +0 lines, -100 lines 0 comments Download
D remoting/client/plugin/chromoting_scriptable_object.cc View 1 chunk +0 lines, -172 lines 1 comment Download
M remoting/client/plugin/pepper_view.cc View 1 chunk +3 lines, -5 lines 0 comments Download
M remoting/client/x11_view.cc View 2 chunks +1 line, -3 lines 0 comments Download
M remoting/remoting.gyp View 1 chunk +0 lines, -2 lines 0 comments Download
Trybot results:
Commit: CQ not working?

Messages

Total messages: 5 (0 generated)
bryeung
4 years, 10 months ago (2010-07-28 15:39:29 UTC) #1
kobi
pp:Var has nothing to do with the error. typedef pp::Var (ChromotingScriptableObject::*MethodHandler)( const std::vector<pp::Var>& args, pp::Var* ...
4 years, 10 months ago (2010-07-28 18:10:18 UTC) #2
bryeung1
Thanks for the suggestion. I'm just sheriff, so I'll let ajwong sort this out when ...
4 years, 10 months ago (2010-07-28 18:12:27 UTC) #3
awong (On leave)
I am out for a few days. Gary...could I trouble to fix this for me ...
4 years, 10 months ago (2010-07-28 19:52:11 UTC) #4
awong (On leave)
4 years, 10 months ago (2010-07-29 00:33:18 UTC) #5
Just verified that Kobic is right in that comparing versus NULL does not
work here.  I think this sounds like a compiler error.  Added a TODO to
check later and submitting with 0 swapped in for NULL.

Thanks for the tip!

-Albert


On Wed, Jul 28, 2010 at 12:52 PM, Albert J. Wong (王重傑)
<ajwong@chromium.org>wrote:

> I am out for a few days.  Gary...could I trouble to fix this for me and
> resubmit?  I'm guessing it should be easy.  As for null versus 0, iirc c++
> defines in the spec that NULL is exactly the same as the literal 0.  But
> maybe I am wrong.
>
> -Albert
>
> On Jul 28, 2010 11:12 AM, "Bryan Yeung" <bryeung@google.com> wrote:
> > Thanks for the suggestion.
> >
> > I'm just sheriff, so I'll let ajwong sort this out when he comes online.
> >
> > Bryan
> >
> > On Wed, Jul 28, 2010 at 11:10 AM, <kobic@codeaurora.org> wrote:
> >> pp:Var has nothing to do with the error.
> >> typedef pp::Var (ChromotingScriptableObject::*MethodHandler)(
> >> const std::vector<pp::Var>& args, pp::Var* exception);
> >>
> >> Is just like
> >> typedef RET_VAL (CLASS::*type_name) (arg1, arg2, ...)
> >>
> >> So the following should be legal:
> >>
> >> type_name foo = 0;
> >>
> >> Maybe arm does not like NULL. Basically, I always use 0 and not NULL to
> be
> >> cross
> >> platform compatible.
> >>
> >>
> >> http://codereview.chromium.org/2856067/diff/1/7
> >> File remoting/client/plugin/chromoting_scriptable_object.cc (left):
> >>
> >> http://codereview.chromium.org/2856067/diff/1/7#oldcode48
> >> remoting/client/plugin/chromoting_scriptable_object.cc:48: return
> >> properties_[iter->second].method == NULL;
> >> That one should be a valid statement. I would use 0 instead of NULL and
> >> see if it makes things better.
> >>
> >> http://codereview.chromium.org/2856067/show
> >>
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld ec887be