Bug 103979 - [V8] V8Binding::v8ExternalString() is redundant
Summary: [V8] V8Binding::v8ExternalString() is redundant
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:
 
Reported: 2012-12-04 00:45 PST by Kentaro Hara
Modified: 2012-12-04 20:26 PST (History)
3 users (show)

See Also:


Attachments
Patch (8.79 KB, patch)
2012-12-04 00:47 PST, 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-12-04 00:45:47 PST
v8String(), v8StringOrNull() and v8StringOrUndefined() are enough. We can remove V8Binding::v8ExternalString().
Comment 1 Kentaro Hara 2012-12-04 00:47:59 PST
Created attachment 177436 [details]
Patch
Comment 2 Adam Barth 2012-12-04 10:17:22 PST
Comment on attachment 177436 [details]
Patch

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

> Source/WebCore/bindings/v8/V8Binding.h:107
> +        return str.isNull() ? v8::Handle<v8::Value>(v8::Null()) : v8::Handle<v8::Value>(v8String(str, isolate));

Shouldn't we pass the isolate to v8Null(isolate) here?
Comment 3 Kentaro Hara 2012-12-04 15:39:46 PST
(In reply to comment #2)
> (From update of attachment 177436 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=177436&action=review
> 
> > Source/WebCore/bindings/v8/V8Binding.h:107
> > +        return str.isNull() ? v8::Handle<v8::Value>(v8::Null()) : v8::Handle<v8::Value>(v8String(str, isolate));
> 
> Shouldn't we pass the isolate to v8Null(isolate) here?

Yeah, I'll fix it when I replace v8::Null(isolate) with v8Null(isolate).
Comment 4 WebKit Review Bot 2012-12-04 20:04:57 PST
Comment on attachment 177436 [details]
Patch

Rejecting attachment 177436 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
ripts/update-webkit line 152.

Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 2
Updating OpenSource
From http://git.chromium.org/external/Webkit
   035dd10..6502679  HEAD       -> origin/HEAD
error: Ref refs/remotes/origin/master is at 6502679cc4e791c06a9f646d2448d49b53086645 but expected 035dd105cd1e6e7b8fc205519a1d095e9c4b4b20
 ! 035dd10..6502679  master     -> origin/master  (unable to update local ref)
Died at Tools/Scripts/update-webkit line 152.

Full output: http://queues.webkit.org/results/15159076
Comment 5 Adam Barth 2012-12-04 20:11:32 PST
Comment on attachment 177436 [details]
Patch

Sorry.  I switched the bot to git.webkit.org.  Hopefully that will help.
Comment 6 WebKit Review Bot 2012-12-04 20:26:46 PST
Comment on attachment 177436 [details]
Patch

Clearing flags on attachment: 177436

Committed r136623: <http://trac.webkit.org/changeset/136623>
Comment 7 WebKit Review Bot 2012-12-04 20:26:50 PST
All reviewed patches have been landed.  Closing bug.