Bug 24664 - Need to upstream V8Collection.h
Summary: Need to upstream V8Collection.h
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: David Levin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-17 18:54 PDT by David Levin
Modified: 2009-03-18 13:11 PDT (History)
1 user (show)

See Also:


Attachments
Proposed fix. (11.33 KB, patch)
2009-03-17 18:58 PDT, David Levin
dglazkov: review-
Details | Formatted Diff | Diff
Proposed fix. (11.52 KB, patch)
2009-03-18 10:54 PDT, David Levin
dglazkov: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Levin 2009-03-17 18:54:48 PDT
See summary.
Comment 1 David Levin 2009-03-17 18:58:26 PDT
Created attachment 28715 [details]
Proposed fix.
Comment 2 Dimitri Glazkov (Google) 2009-03-18 09:23:28 PDT
Comment on attachment 28715 [details]
Proposed fix.

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 7c6972d..3890a71 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -2,6 +2,17 @@
>  
>          Reviewed by NOBODY (OOPS!).
>  
> +        <https://bugs.webkit.org/show_bug.cgi?id=24664>
> +        Upstreaming v8 collection.h
> +
> +        No change in behavior, so no test.
> +
> +        * bindings/v8/V8Collection.h: Added.
> +
> +2009-03-17  David Levin  <levin@chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
>          <https://bugs.webkit.org/show_bug.cgi?id=24662>
>          Chromium build fixes.
>          
> diff --git a/WebCore/bindings/v8/V8Collection.h b/WebCore/bindings/v8/V8Collection.h
> new file mode 100644
> index 0000000..4b2d672
> --- /dev/null
> +++ b/WebCore/bindings/v8/V8Collection.h
> @@ -0,0 +1,191 @@
> +/*
> + * Copyright (C) 2006, 2007, 2008, 2009 Google Inc. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are
> + * met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above
> + * copyright notice, this list of conditions and the following disclaimer
> + * in the documentation and/or other materials provided with the
> + * distribution.
> + *     * Neither the name of Google Inc. nor the names of its
> + * contributors may be used to endorse or promote products derived from
> + * this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef V8Collection_h
> +#define V8Collection_h
> +
> +#include "V8Binding.h"
> +#include "V8Proxy.h"
> +#include <v8.h>
> +
> +namespace WebCore {
> +
> +    static v8::Handle<v8::Value> getV8Object(void* implementation, v8::Local<v8::Value> implementationType)

getXXX is usually applied when the value is returned via a ref argument, i.e. bool getName(Type& returnValue);
In this particular case, you should probably just use toV8(..), which is a convention I stole from JSC bindings.

Oh wait, this is just cleaning up of the old code. If you don't feel like more scrubbing, add a FIXME for this suggestion.
Comment 3 David Levin 2009-03-18 10:26:48 PDT
I went through all of the names and came up with this possible renaming:

getV8Object                                   -> toV8Object
getNamedPropertyOfCollection                  -> toNamedPropertyOfCollection
collectionNamedPropertyGetter                 -> toNamedPropertyOfCollection
nodeCollectionNamedPropertyGetter             -> toNamedPropertyOfCollection
getIndexedPropertyOfCollection                -> toIndexedPropertyOfCollection
collectionIndexedPropertyGetter               -> toIndexedPropertyOfCollection
nodeCollectionIndexedPropertyGetter           -> toIndexedPropertyOfCollection
nodeCollectionIndexedPropertyEnumerator       -> toIndexedPropertyArrayOfNode
collectionIndexedPropertyEnumerator           -> toIndexedPropertyArrayOfCollection
collectionStringOrNullIndexedPropertyGetter   -> toStringOrNullOfCollection

setCollectionIndexedGetter                    -> setIndexedGetterForCollection
setCollectionNamedGetter                      -> setNamedGetterForCollection
setCollectionIndexedAndNamedGetters           -> setIndexedAndNamedGettersForCollection
setCollectionStringOrNullIndexedGetter        -> setStringOrNullIndexedGetterForCollection


What do you think?
Comment 4 David Levin 2009-03-18 10:54:12 PDT
Created attachment 28725 [details]
Proposed fix.

Added fixme as suggested.
Comment 5 Dimitri Glazkov (Google) 2009-03-18 11:29:26 PDT
Comment on attachment 28725 [details]
Proposed fix.

Ok.
Comment 6 David Levin 2009-03-18 13:11:04 PDT
Commited as r41812.