Bug 93115 - [V8] Remove V8Proxy::registerExtensionWithV8()
Summary: [V8] Remove V8Proxy::registerExtensionWithV8()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks: 85330
  Show dependency treegraph
 
Reported: 2012-08-03 07:13 PDT by Kentaro Hara
Modified: 2012-08-05 08:40 PDT (History)
3 users (show)

See Also:


Attachments
Patch (3.69 KB, patch)
2012-08-03 07:15 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff
patch for landing (3.80 KB, patch)
2012-08-05 07:04 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2012-08-03 07:13:30 PDT
V8Proxy::registerExtensionWithV8() is used by V8Proxy::registerExtension() only. We can remove the redundant method.

In a follow-up patch, I will implement V8Proxy::registerExtentionIfNeeded() and remove registerExtensionWithV8() and registerExtension().
Comment 1 Kentaro Hara 2012-08-03 07:15:52 PDT
Created attachment 156369 [details]
Patch
Comment 2 Adam Barth 2012-08-03 09:55:55 PDT
Comment on attachment 156369 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=156369&action=review

> Source/WebCore/bindings/v8/V8Proxy.cpp:678
> +const V8Extensions& V8Proxy::extensions()

Why have both staticExtensionsList() and V8Proxy::extensions() ?
Comment 3 Kentaro Hara 2012-08-03 09:59:03 PDT
(In reply to comment #2)
> (From update of attachment 156369 [details])
> > Source/WebCore/bindings/v8/V8Proxy.cpp:678
> > +const V8Extensions& V8Proxy::extensions()
> 
> Why have both staticExtensionsList() and V8Proxy::extensions() ?

- staticExtensionsList() is not const and can be used by V8Proxy.cpp only.

- extensions() is const and can be used by outside (i.e. V8DOMWindowShell.cpp)

Shall we merge them by removing const?
Comment 4 Adam Barth 2012-08-03 10:23:18 PDT
> Shall we merge them by removing const?

I'm not sure it's a big deal either way.  I leave it up to your judgement.
Comment 5 Kentaro Hara 2012-08-03 10:24:52 PDT
(In reply to comment #4)
> > Shall we merge them by removing const?
> 
> I'm not sure it's a big deal either way.  I leave it up to your judgement.

OK, let me merge them and land it (after depending patches are landed).
Comment 6 Kentaro Hara 2012-08-05 07:04:01 PDT
Created attachment 156558 [details]
patch for landing
Comment 7 WebKit Review Bot 2012-08-05 08:39:45 PDT
Comment on attachment 156558 [details]
patch for landing

Clearing flags on attachment: 156558

Committed r124716: <http://trac.webkit.org/changeset/124716>