We can move V8Parameter, V8ParameterBase and String related code in V8Binding to a separate file.
Created attachment 159618 [details] Patch
I'll do the following things in follow-up patches: - Remove V8ParameterBase. It's redundant. Only V8Parameter will work. - Rename V8Parameter to V8GenericString, because it's an adapter class for String and AtomicString.
Comment on attachment 159618 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159618&action=review > Source/WebCore/bindings/v8/V8StringResource.cpp:35 > + I'd remove this blank line. > Source/WebCore/bindings/v8/V8StringResource.cpp:52 > + String result = String::createUninitialized(length, buffer); Does this really use int as the type for length? > Source/WebCore/bindings/v8/V8StringResource.cpp:75 > + String tmp = String::createUninitialized(length, buffer); tmp -> string ? > Source/WebCore/bindings/v8/V8StringResource.cpp:88 > + int length = v8String->Length(); Oh, V8 uses int for the length of a string? > Source/WebCore/bindings/v8/V8StringResource.cpp:91 > + return StringImpl::empty(); Is this ok for background threads? Typically we can't use these sorts of functions if we're not on the main thread.
Created attachment 159640 [details] patch for landing
Committed r126150: <http://trac.webkit.org/changeset/126150>
(In reply to comment #3) > (From update of attachment 159618 [details]) > > Source/WebCore/bindings/v8/V8StringResource.cpp:35 > > + > > I'd remove this blank line. > > > Source/WebCore/bindings/v8/V8StringResource.cpp:75 > > + String tmp = String::createUninitialized(length, buffer); > > tmp -> string ? > > > Source/WebCore/bindings/v8/V8StringResource.cpp:91 > > + return StringImpl::empty(); > > Is this ok for background threads? Typically we can't use these sorts of functions if we're not on the main thread. Done. Replaced with String().
Re-opened since this is blocked by 94605
Created attachment 159852 [details] patch for landing
(In reply to comment #6) > > > Source/WebCore/bindings/v8/V8StringResource.cpp:91 > > > + return StringImpl::empty(); > > > > Is this ok for background threads? Typically we can't use these sorts of functions if we're not on the main thread. > > Replaced with String(). This was the cause of the crashes. I replaced it with String(""). Given that this is not a fast path of v8String(), it would be OK to create String("") every time.
Comment on attachment 159852 [details] patch for landing Clearing flags on attachment: 159852 Committed r126262: <http://trac.webkit.org/changeset/126262>
All reviewed patches have been landed. Closing bug.