[Webkit-unassigned] [Bug 35634] [DRT/Chromium] Add CppVariant and CppBoundClass

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=35634





--- Comment #9 from TAMURA, Kent <tkent at chromium.org>  2010-03-03 23:43:01 PST ---
> > +// Copyright (c) 2010 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.
> 
> Incorrect license header. Please use standard WebKit header.

I replaced them with the standard one with "The Chromium Authors".

> > +    scoped_ptr<CppBoundClass::GetterCallback> m_callback;
> 
> Since we're using WTF already, let's use OwnPtr here.

Done.

> > +    static bool invoke(NPObject*, NPIdentifier,
> > +                       const NPVariant* args, uint32_t arg_count,
> > +                       NPVariant* result);
> 
> arguments, argumentCount

Done.  Fixed other occurrences.

> > +/* static */
> 
> No need for these comments in WebKit-land.

Done.

> > +    PropertyCallback* propertyCallback =
> > +        callback ? new GetterPropertyCallback(callback) : 0;
> 
> Let's just keep these on one line.

Done.

> > +    PropertyCallback* propertyCallback =
> > +        prop ? new CppVariantPropertyCallback(prop) : 0;
> 
> Ditto.

Ditto.

> > +#if USE(JSC)
> > +#error "This is not going to work anymore...but it's not clear what the solution is...or if it's still necessary."
> > +    JSC::JSLock lock(false);
> > +#endif
> 
> This is not needed here.

Removed.

> > +    frame->bindToWindowObject(classname,
> > +                              NPVARIANT_TO_OBJECT(*getAsCppVariant()));
> 
> Keep on one line.

Done.

> > +#include "base/callback.h"
> 
> This dependency can be broken fairly easily. We only use a very small subset of
> Chromium callback machinery.

Ok, I made specialized callback classes and put them into this file.

> > +#include "base/scoped_ptr.h"
> 
> As mentioned before, might be good to break this dependency and use
> wtf/OwnPtr.h

Done.

> > +#include <map>
> > +#include <vector>
> 
> Can use wtf primitives here as well.

Done.

> > +    DISALLOW_COPY_AND_ASSIGN(CppBoundClass);
> 
> Can use wtf/Noncopyable

Done.

> > +
> > +// This file contains definitions for CppVariant.
> > +
> 
> Gratuitous comment :)

Removed.

> > +  For a usage example, see cpp_binding_example.{h|cc}.
> 
> Rename this?

Removed.

> > +#include "third_party/npapi/bindings/npruntime.h"
> 
> This seems odd. How will this build here?

I supposed WebKit/chromium was in the include paths and third_party/npapi was
extracted to there.
I changed it to WebBindings.h.  It seems better.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.