See summary.
Created attachment 30479 [details] patch
Comment on attachment 30479 [details] patch Lots of style nits. Please make sure this still compiles after addressing. > +v8::Handle<v8::Value> V8Custom::WindowSetTimeoutImpl(const v8::Arguments& args, bool single_shot) singleShot > +{ > + int num_arguments = args.Length(); argumentCount? > + ScriptExecutionContext* script_context = static_cast<ScriptExecutionContext*>(imp->frame()->document()); scriptContext > + WebCore::String string_function = ToWebCoreString(function); functionString > + int param_count = num_arguments >= 2 ? num_arguments - 2 : 0; paramCount > +static bool IsAscii(const String& str) > +{ > + for (size_t i = 0; i < str.length(); i++) { > + if (str[i] > 0xFF) > + return false; > + } > + return true; > +} I believe there's already an isAscii helper somewhere. Please look for it and reuse. At the very least, put a FIXME to do that :) > + Vector<char> in(str.length()); inputCharacters > + Vector<char> out; outputCharacters > +// TODO(fqian): returning string is cheating, and we should FIXME: is the WebKit-speak for TODO(name): > +static String EventNameFromAttributeName(const String& name) eventNameFromAttributeName > + String event_type = name.substring(2); eventType > + String event_type = EventNameFromAttributeName(key); eventType > + if (value->IsNull()) > + // Clear the event listener > + imp->clearAttributeEventListener(event_type); Need braces. I know this is the opposite of what Darin said, but it's actually an amendment to the rule. No braces on one-liners UNLESS there are comments. > + String event_type = EventNameFromAttributeName(key); eventType > + DOMWindow* target_win = V8Proxy::ToNativeObject<DOMWindow>(V8ClassIndex::DOMWINDOW, window); targetWindow > + DOMWindow* target_win = V8Proxy::ToNativeObject<DOMWindow>(V8ClassIndex::DOMWINDOW, window); targetWindow
Created attachment 30485 [details] patch2
All requested changes made, except that I couldn't find a suitable replacement for IsAscii().
Comment on attachment 30485 [details] patch2 Darn it! One more round of laundering :-\ > + V8Proxy::ThrowError(V8Proxy::GENERAL_ERROR, "Cannot decode base64"); > + return v8::Undefined(); One more! Use throwError helper from V8Proxy.h > + if (args.Length() < 1) { > + V8Proxy::ThrowError(V8Proxy::SYNTAX_ERROR, "Not enough arguments"); > + return v8::Undefined(); > + } Same here. > + if (args.Length() < 1) { > + V8Proxy::ThrowError(V8Proxy::SYNTAX_ERROR, "Not enough arguments"); > + return v8::Undefined(); > + } And here. > + String str = ToWebCoreString(args[0]); toWebCoreString > + return Base64Convert(str, true); Whoops. Functions should be camelCase. How did I miss this earlier? convertBase64?
Created attachment 30511 [details] patch3
Created attachment 30512 [details] patch4
Comment on attachment 30512 [details] patch4 Wee!
http://trac.webkit.org/changeset/43936