Bug 119548

Summary: Refactoring of throw exception.
Product: WebKit Reporter: Chris Curtis <chris_curtis>
Component: JavaScriptCoreAssignee: Chris Curtis <chris_curtis>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, buildbot, commit-queue, dino, eflews.bot, eric.carlson, ggaren, glenn, gtk-ews, gyuyoung.kim, jer.noble, jsbell, kondapallykalyan, mkwst, philn, rego+ews, rniwa, webkit-ews, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
ggaren: review-, webkit-ews: commit-queue-
patch2
none
patch3
ggaren: review-, webkit-ews: commit-queue-
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion
none
patch3
eflews.bot: commit-queue-
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion
none
patch 4
webkit-ews: commit-queue-
patch 5
gtk-ews: commit-queue-
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2
none
patch 6
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
none
patch 7
ggaren: review-, ggaren: commit-queue-
patch 7 followup
chris_curtis: review-, chris_curtis: commit-queue-
patch 8 none

Description Chris Curtis 2013-08-07 08:22:42 PDT 
The act of throwing an exception is being handled in different ways depending on whether the code was running in the LLint, Baseline JIT, or the DFG JIT. Also there are redundant attempts to add error info. This has the ability to slow down the system.
Comment 1 Chris Curtis 2013-08-07 08:49:46 PDT 
Created attachment 208271 [details]
patch

Initial review. I still need to modify compile for to use the new throw exception. Also generic throw is clearing the exception for a specific case inside unwind exception that needs to be modified.
Comment 2 WebKit Commit Bot 2013-08-07 08:52:36 PDT 
Attachment 208271 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/events/window-onerror5-expected.txt', u'LayoutTests/fast/js/mozilla/resources/js-test-pre.js', u'LayoutTests/http/tests/workers/worker-importScriptsOnError-expected.txt', u'Source/JavaScriptCore/API/APICallbackFunction.h', u'Source/JavaScriptCore/API/JSCallbackConstructor.cpp', u'Source/JavaScriptCore/API/JSCallbackObjectFunctions.h', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/debugger/Debugger.cpp', u'Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp', u'Source/JavaScriptCore/dfg/DFGAssemblyHelpers.h', u'Source/JavaScriptCore/dfg/DFGOperations.cpp', u'Source/JavaScriptCore/heap/Heap.cpp', u'Source/JavaScriptCore/interpreter/CallFrame.h', u'Source/JavaScriptCore/interpreter/Interpreter.cpp', u'Source/JavaScriptCore/interpreter/Interpreter.h', u'Source/JavaScriptCore/jit/JITExceptions.cpp', u'Source/JavaScriptCore/jit/JITOpcodes.cpp', u'Source/JavaScriptCore/jit/JITOpcodes32_64.cpp', u'Source/JavaScriptCore/jit/JITStubs.cpp', u'Source/JavaScriptCore/jsc.cpp', u'Source/JavaScriptCore/llint/LLIntSlowPaths.cpp', u'Source/JavaScriptCore/runtime/ArrayConstructor.cpp', u'Source/JavaScriptCore/runtime/CommonSlowPaths.cpp', u'Source/JavaScriptCore/runtime/CommonSlowPaths.h', u'Source/JavaScriptCore/runtime/Completion.cpp', u'Source/JavaScriptCore/runtime/Error.cpp', u'Source/JavaScriptCore/runtime/Error.h', u'Source/JavaScriptCore/runtime/ExceptionHelpers.cpp', u'Source/JavaScriptCore/runtime/Executable.cpp', u'Source/JavaScriptCore/runtime/FunctionConstructor.cpp', u'Source/JavaScriptCore/runtime/JSArray.cpp', u'Source/JavaScriptCore/runtime/JSCJSValue.cpp', u'Source/JavaScriptCore/runtime/JSFunction.cpp', u'Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp', u'Source/JavaScriptCore/runtime/JSNameScope.cpp', u'Source/JavaScriptCore/runtime/JSONObject.cpp', u'Source/JavaScriptCore/runtime/JSObject.cpp', u'Source/JavaScriptCore/runtime/ObjectConstructor.cpp', u'Source/JavaScriptCore/runtime/RegExpConstructor.cpp', u'Source/JavaScriptCore/runtime/StringObject.cpp', u'Source/JavaScriptCore/runtime/StringRecursionChecker.cpp', u'Source/JavaScriptCore/runtime/VM.cpp', u'Source/JavaScriptCore/runtime/VM.h', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/js/JSArrayBufferViewHelper.h', u'Source/WebCore/bindings/js/JSAudioBufferSourceNodeCustom.cpp', u'Source/WebCore/bindings/js/JSBiquadFilterNodeCustom.cpp', u'Source/WebCore/bindings/js/JSCryptoCustom.cpp', u'Source/WebCore/bindings/js/JSDOMBinding.cpp', u'Source/WebCore/bindings/js/JSDataViewCustom.cpp', u'Source/WebCore/bindings/js/JSInjectedScriptHostCustom.cpp', u'Source/WebCore/bindings/js/JSJavaScriptCallFrameCustom.cpp', u'Source/WebCore/bindings/js/JSNodeFilterCondition.cpp', u'Source/WebCore/bindings/js/JSOscillatorNodeCustom.cpp', u'Source/WebCore/bindings/js/JSPannerNodeCustom.cpp', u'Source/WebCore/bindings/js/JSSVGLengthCustom.cpp', u'Source/WebCore/bindings/js/JSWebGLRenderingContextCustom.cpp', u'Source/WebCore/bindings/js/JSXMLHttpRequestCustom.cpp', u'Source/WebCore/bindings/js/ScriptFunctionCall.cpp', u'Source/WebCore/bindings/js/SerializedScriptValue.cpp', u'Source/WebCore/bindings/js/WorkerScriptController.cpp', u'Source/WebCore/bridge/c/c_instance.cpp', u'Source/WebCore/bridge/objc/objc_instance.mm', u'Source/WebCore/bridge/objc/objc_runtime.mm', u'Source/WebCore/bridge/objc/objc_utility.mm', u'Source/WebCore/bridge/runtime_array.cpp', u'Source/WebCore/bridge/runtime_object.cpp', u'Source/WebKit/mac/ChangeLog', u'Source/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm', u'Source/WebKit/mac/Plugins/Hosted/ProxyInstance.mm', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/Plugins/Netscape/JSNPObject.cpp', u'Source/WebKit2/WebProcess/Plugins/Netscape/NPRuntimeObjectMap.cpp']" exit_code: 1
Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:153:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 73 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Early Warning System Bot 2013-08-07 08:58:34 PDT 
Comment on attachment 208271 [details]
patch

Attachment 208271 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1376303
Comment 4 Early Warning System Bot 2013-08-07 09:01:39 PDT 
Comment on attachment 208271 [details]
patch

Attachment 208271 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1384285
Comment 5 Geoffrey Garen 2013-08-07 09:12:43 PDT 
Looks like you need to update this non-Mac file:

/home/webkit/WebKit/Source/WebCore/bridge/qt/qt_instance.cpp: In member function 'virtual JSC::JSValue JSC::Bindings::QtField::valueFromInstance(JSC::ExecState*, const JSC::Bindings::Instance*) const':
/home/webkit/WebKit/Source/WebCore/bridge/qt/qt_instance.cpp:340:58: error: 'throwError' was not declared in this scope
/home/webkit/WebKit/Source/WebCore/bridge/qt/qt_instance.cpp:344:74: error: 'throwError' was not declared in this scope
Comment 6 Geoffrey Garen 2013-08-07 09:22:07 PDT 
Comment on attachment 208271 [details]
patch

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

> Source/JavaScriptCore/ChangeLog:44
> +        (JSC::Interpreter::addStackTraceIfNecessary): Removed this because it is never necessary now.

Did you remove this function? It seems to be present still.

What about appendSourceToError and addErrorInfo? Can those move into VM::throwError?

> Source/JavaScriptCore/interpreter/Interpreter.cpp:590
> +NEVER_INLINE HandlerInfo* Interpreter::unwindException(CallFrame*& callFrame, JSValue& exceptionValue, unsigned bytecodeOffset)

Let's call this "unwind". We're unwinding the stack, not the exception, so "unwind exception" is an awkward phrase.

> Source/JavaScriptCore/runtime/VM.h:341
>          JS_EXPORT_PRIVATE void clearExceptionStack();

Can we remove this function? Clearing the exception stack should be tied to clearing the exception. We want exception and exceptionStack to be tied together.

> Source/JavaScriptCore/runtime/VM.h:346
> +        JS_EXPORT_PRIVATE JSValue throwError(ExecState*, JSValue);
> +        JS_EXPORT_PRIVATE JSObject* throwError(ExecState*, JSObject*);

Let's call these "throwException", to be consistent in our terminology.

> Source/JavaScriptCore/runtime/VM.h:351
> +        JSValue exception;

Let's make this value private, and add an exception() accessor.

JIT code that used to use OBJECT_OFFSETOF(struct JITStackFrame, vm) can use a member function instead. See JSCell::structureOffset() for an example.

> Source/WebCore/bindings/js/ScriptFunctionCall.cpp:-128
>      JSLockHolder lock(m_exec);
> -
>      JSValue function = thisObject->get(m_exec, Identifier(m_exec, m_name));
>      if (m_exec->hadException()) {
>          if (reportExceptions)
>              reportException(m_exec, m_exec->exception());
> -

Please revert these whitespace changes to maintain a clean svn history.

> LayoutTests/ChangeLog:13
> +        These tests have only the column number changed.
> +        * fast/events/window-onerror5-expected.txt:
> +        * http/tests/workers/worker-importScriptsOnError-expected.txt:

Is the new column number better? If so, why?

> LayoutTests/ChangeLog:72
> -2013-08-06  Filip Pizlo  <fpizlo@apple.com>
> +013-08-06  Filip Pizlo  <fpizlo@apple.com>

Please revert.
Comment 7 Chris Curtis 2013-08-07 09:48:09 PDT 
(In reply to comment #6)
> (From update of attachment 208271 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=208271&action=review
> 
> > Source/JavaScriptCore/ChangeLog:44
> > +        (JSC::Interpreter::addStackTraceIfNecessary): Removed this because it is never necessary now.
> 
> Did you remove this function? It seems to be present still.
I missed the header. It is removed now.
> What about appendSourceToError and addErrorInfo? Can those move into VM::throwError?
> 
They should be, the only information that is missing is the byte code offset.
Comment 8 Chris Curtis 2013-08-16 12:17:58 PDT 
Created attachment 208943 [details]
patch2
Comment 9 Darin Adler 2013-08-16 12:48:33 PDT 
Comment on attachment 208943 [details]
patch2

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

> Source/JavaScriptCore/runtime/VM.cpp:597
> +    VM* vm = &exec->vm();

Why does this code call &exec->vm() instead of using "this"? If it doesn’t use "this" then it should be a static member function, not a member function.

> Source/JavaScriptCore/runtime/VM.cpp:603
> +    exec->vm().m_exception = error;

And now here we are using exec->vm() directly instead of the local variable "vm" or "this".
Comment 10 Geoffrey Garen 2013-08-16 13:34:54 PDT 
Comment on attachment 208943 [details]
patch2

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

> Source/JavaScriptCore/ChangeLog:16
> +        * API/APICallbackFunction.h:
> +        (JSC::APICallbackFunction::call):

In this listing, changes that require explanation should include an explanation, and changes that don't should not be listed at all.

> Source/JavaScriptCore/interpreter/Interpreter.cpp:538
> +        // We need to clear the exception stack here inorder to see if a new exception happens.

Should be: "in order".

We clear the exception and the exception stack.

> Source/JavaScriptCore/interpreter/Interpreter.cpp:540
> +        // Afterwards, the values will be put back to continue processing this error.
> +        exceptionHolder holder(&callFrame->vm());

It would be nice to give this object a name that matched your description of what it's for. Since you said "clear the exception", how about calling this "ClearExceptionScope scope(&callFrame->vm())"?

> Source/JavaScriptCore/interpreter/Interpreter.h:137
> +    class exceptionHolder {

Class names are capitalized in WebKit.

> Source/JavaScriptCore/interpreter/Interpreter.h:141
> +            vm->exceptionInfo(oldException, oldExceptionStack);

Functions with out parameters are prefixed with "get" in WebKit: vm->getExceptionInfo(...);
Comment 11 Chris Curtis 2013-08-20 16:56:24 PDT 
Created attachment 209237 [details]
patch3
Comment 12 Early Warning System Bot 2013-08-20 17:07:01 PDT 
Comment on attachment 209237 [details]
patch3

Attachment 209237 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1503984
Comment 13 Early Warning System Bot 2013-08-20 17:08:22 PDT 
Comment on attachment 209237 [details]
patch3

Attachment 209237 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1500967
Comment 14 EFL EWS Bot 2013-08-20 17:25:03 PDT 
Comment on attachment 209237 [details]
patch3

Attachment 209237 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1500966
Comment 15 EFL EWS Bot 2013-08-20 17:28:14 PDT 
Comment on attachment 209237 [details]
patch3

Attachment 209237 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1513730
Comment 16 Geoffrey Garen 2013-08-20 17:41:09 PDT 
Looks like you're missing some StackFrame #includes:

home/webkit/WebKit/Source/JavaScriptCore/bytecode/SamplingTool.cpp
In file included from /home/webkit/WebKit/Source/JavaScriptCore/runtime/VM.h:57:0,
                 from /home/webkit/WebKit/Source/JavaScriptCore/assembler/AssemblerBuffer.h:33,
                 from /home/webkit/WebKit/Source/JavaScriptCore/assembler/X86Assembler.h:31,
                 from /home/webkit/WebKit/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:31,
                 from /home/webkit/WebKit/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.cpp:29:
/home/webkit/WebKit/Source/WTF/wtf/RefCountedArray.h: In member function 'T* WTF::RefCountedArray<T>::end() [with T = JSC::StackFrame]':
/home/webkit/WebKit/Source/WTF/wtf/RefCountedArray.h:111:9:   instantiated from 'WTF::RefCountedArray<T>::~RefCountedArray() [with T = JSC::StackFrame]'
/home/webkit/WebKit/Source/JavaScriptCore/runtime/VM.h:344:69:   instantiated from here
/home/webkit/WebKit/Source/WTF/wtf/RefCountedArray.h:128:54: error: invalid use of incomplete type 'struct JSC::StackFrame'
/home/webkit/WebKit/Source/JavaScriptCore/runtime/VM.h:85:12: error: forward declaration of 'struct JSC::StackFrame'
In file included from /home/webkit/WebKit/Source/JavaScriptCore/jit/ExecutableAllocator.h:37:0,
                 from /home/webkit/WebKit/Source/JavaScriptCore/assembler/AssemblerBuffer.h:31,
                 from /home/webkit/WebKit/Source/JavaScriptCore/assembler/X86Assembler.h:31,
                 from /home/webkit/WebKit/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:31,
                 from /home/webkit/WebKit/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.cpp:29:
/home/webkit/WebKit/Source/WTF/wtf/Vector.h: In static member function 'static void WTF::VectorDestructor<true, T>::destruct(T*, T*) [with T = JSC::StackFrame]':
/home/webkit/WebKit/Source/WTF/wtf/Vector.h:214:13:   instantiated from 'static void WTF::VectorTypeOperations<T>::destruct(T*, T*) [with T = JSC::StackFrame]'
/home/webkit/WebKit/Source/WTF/wtf/RefCountedArray.h:111:9:   instantiated from 'WTF::RefCountedArray<T>::~RefCountedArray() [with T = JSC::StackFrame]'
/home/webkit/WebKit/Source/JavaScriptCore/runtime/VM.h:344:69:   instantiated from here
/home/webkit/WebKit/Source/WTF/wtf/Vector.h:51:27: error: cannot increment a pointer to incomplete type 'JSC::StackFrame'
/home/webkit/WebKit/Source/WTF/wtf/Vector.h:52:17: error: invalid use of incomplete type 'struct JSC::StackFrame'
/home/webkit/WebKit/Source/JavaScriptCore/runtime/VM.h:85:12: error: forward declaration of 'struct JSC::StackFrame'
Comment 17 kov's GTK+ EWS bot 2013-08-20 18:06:21 PDT 
Comment on attachment 209237 [details]
patch3

Attachment 209237 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1515709
Comment 18 Build Bot 2013-08-21 03:30:34 PDT 
Comment on attachment 209237 [details]
patch3

Attachment 209237 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1520475

New failing tests:
inspector/debugger/pause-in-inline-script.html
inspector/debugger/debugger-pause-in-internal.html
inspector/debugger/debugger-pause-on-exception.html
inspector-protocol/debugger-setVariableValue.html
inspector/debugger/watch-expressions-panel-switch.html
inspector/debugger/debugger-change-variable.html
fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html
Comment 19 Build Bot 2013-08-21 03:30:38 PDT 
Created attachment 209258 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.4
Comment 20 Geoffrey Garen 2013-08-21 09:12:11 PDT 
Comment on attachment 209237 [details]
patch3

Looks like some tests are failing, too.
Comment 21 Chris Curtis 2013-08-26 17:04:55 PDT 
Created attachment 209695 [details]
patch3

Not sure I got all the #includes on the other ports
Comment 22 EFL EWS Bot 2013-08-26 17:13:30 PDT 
Comment on attachment 209695 [details]
patch3

Attachment 209695 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1579085
Comment 23 Early Warning System Bot 2013-08-26 17:13:59 PDT 
Comment on attachment 209695 [details]
patch3

Attachment 209695 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1582102
Comment 24 Early Warning System Bot 2013-08-26 17:14:16 PDT 
Comment on attachment 209695 [details]
patch3

Attachment 209695 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1587081
Comment 25 EFL EWS Bot 2013-08-26 17:24:46 PDT 
Comment on attachment 209695 [details]
patch3

Attachment 209695 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1589074
Comment 26 kov's GTK+ EWS bot 2013-08-26 17:26:36 PDT 
Comment on attachment 209695 [details]
patch3

Attachment 209695 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1578104
Comment 27 Build Bot 2013-08-26 18:37:06 PDT 
Comment on attachment 209695 [details]
patch3

Attachment 209695 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1582118

New failing tests:
fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html
Comment 28 Build Bot 2013-08-26 18:37:10 PDT 
Created attachment 209703 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.4
Comment 29 Chris Curtis 2013-08-27 13:50:21 PDT 
Created attachment 209795 [details]
patch 4

All the Ports should build now. A layout test might still fail because of differing results from wk and wk2
Comment 30 WebKit Commit Bot 2013-08-27 13:53:11 PDT 
Attachment 209795 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/events/window-onerror4-expected.txt', u'LayoutTests/fast/js/exception-properties-expected.txt', u'LayoutTests/fast/js/global-recursion-on-full-stack-expected.txt', u'LayoutTests/fast/js/mozilla/resources/js-test-pre.js', u'LayoutTests/fast/js/script-tests/exception-properties.js', u'LayoutTests/fast/xmlhttprequest/xmlhttprequest-recursive-sync-event-expected.txt', u'Source/JavaScriptCore/API/APICallbackFunction.h', u'Source/JavaScriptCore/API/JSCallbackConstructor.cpp', u'Source/JavaScriptCore/API/JSCallbackObjectFunctions.h', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/debugger/Debugger.cpp', u'Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp', u'Source/JavaScriptCore/dfg/DFGAssemblyHelpers.h', u'Source/JavaScriptCore/dfg/DFGOperations.cpp', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', u'Source/JavaScriptCore/heap/Heap.cpp', u'Source/JavaScriptCore/interpreter/CallFrame.h', u'Source/JavaScriptCore/interpreter/Interpreter.cpp', u'Source/JavaScriptCore/interpreter/Interpreter.h', u'Source/JavaScriptCore/jit/JITCode.cpp', u'Source/JavaScriptCore/jit/JITExceptions.cpp', u'Source/JavaScriptCore/jit/JITOpcodes.cpp', u'Source/JavaScriptCore/jit/JITOpcodes32_64.cpp', u'Source/JavaScriptCore/jit/JITStubs.cpp', u'Source/JavaScriptCore/jit/SlowPathCall.h', u'Source/JavaScriptCore/jit/ThunkGenerators.cpp', u'Source/JavaScriptCore/jsc.cpp', u'Source/JavaScriptCore/llint/LLIntExceptions.cpp', u'Source/JavaScriptCore/llint/LLIntSlowPaths.cpp', u'Source/JavaScriptCore/llint/LowLevelInterpreter.cpp', u'Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm', u'Source/JavaScriptCore/llint/LowLevelInterpreter64.asm', u'Source/JavaScriptCore/runtime/ArrayConstructor.cpp', u'Source/JavaScriptCore/runtime/CommonSlowPaths.cpp', u'Source/JavaScriptCore/runtime/CommonSlowPaths.h', u'Source/JavaScriptCore/runtime/CommonSlowPathsExceptions.cpp', u'Source/JavaScriptCore/runtime/Completion.cpp', u'Source/JavaScriptCore/runtime/Error.cpp', u'Source/JavaScriptCore/runtime/Error.h', u'Source/JavaScriptCore/runtime/ExceptionHelpers.cpp', u'Source/JavaScriptCore/runtime/Executable.cpp', u'Source/JavaScriptCore/runtime/FunctionConstructor.cpp', u'Source/JavaScriptCore/runtime/JSArray.cpp', u'Source/JavaScriptCore/runtime/JSCJSValue.cpp', u'Source/JavaScriptCore/runtime/JSFunction.cpp', u'Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h', u'Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp', u'Source/JavaScriptCore/runtime/JSNameScope.cpp', u'Source/JavaScriptCore/runtime/JSONObject.cpp', u'Source/JavaScriptCore/runtime/JSObject.cpp', u'Source/JavaScriptCore/runtime/ObjectConstructor.cpp', u'Source/JavaScriptCore/runtime/RegExpConstructor.cpp', u'Source/JavaScriptCore/runtime/StringObject.cpp', u'Source/JavaScriptCore/runtime/StringRecursionChecker.cpp', u'Source/JavaScriptCore/runtime/VM.cpp', u'Source/JavaScriptCore/runtime/VM.h', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/js/JSAudioBufferSourceNodeCustom.cpp', u'Source/WebCore/bindings/js/JSBiquadFilterNodeCustom.cpp', u'Source/WebCore/bindings/js/JSCryptoCustom.cpp', u'Source/WebCore/bindings/js/JSDOMBinding.cpp', u'Source/WebCore/bindings/js/JSInjectedScriptHostCustom.cpp', u'Source/WebCore/bindings/js/JSJavaScriptCallFrameCustom.cpp', u'Source/WebCore/bindings/js/JSNodeFilterCondition.cpp', u'Source/WebCore/bindings/js/JSOscillatorNodeCustom.cpp', u'Source/WebCore/bindings/js/JSPannerNodeCustom.cpp', u'Source/WebCore/bindings/js/JSSVGLengthCustom.cpp', u'Source/WebCore/bindings/js/JSWebGLRenderingContextCustom.cpp', u'Source/WebCore/bindings/js/JSXMLHttpRequestCustom.cpp', u'Source/WebCore/bindings/js/ScriptCallStackFactory.cpp', u'Source/WebCore/bindings/js/SerializedScriptValue.cpp', u'Source/WebCore/bindings/js/WorkerScriptController.cpp', u'Source/WebCore/bridge/c/c_instance.cpp', u'Source/WebCore/bridge/objc/objc_instance.mm', u'Source/WebCore/bridge/objc/objc_runtime.mm', u'Source/WebCore/bridge/objc/objc_utility.mm', u'Source/WebCore/bridge/qt/qt_instance.cpp', u'Source/WebCore/bridge/runtime_array.cpp', u'Source/WebCore/bridge/runtime_object.cpp', u'Source/WebKit/mac/ChangeLog', u'Source/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm', u'Source/WebKit/mac/Plugins/Hosted/ProxyInstance.mm', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/Plugins/Netscape/JSNPObject.cpp', u'Source/WebKit2/WebProcess/Plugins/Netscape/NPRuntimeObjectMap.cpp']" exit_code: 1
Source/WebCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 86 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 31 Early Warning System Bot 2013-08-27 14:01:34 PDT 
Comment on attachment 209795 [details]
patch 4

Attachment 209795 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1624031
Comment 32 Early Warning System Bot 2013-08-27 14:03:18 PDT 
Comment on attachment 209795 [details]
patch 4

Attachment 209795 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1591352
Comment 33 Chris Curtis 2013-08-27 14:10:33 PDT 
Created attachment 209800 [details]
patch 5

more work on the qt port.
Comment 34 kov's GTK+ EWS bot 2013-08-27 14:59:07 PDT 
Comment on attachment 209800 [details]
patch 5

Attachment 209800 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1592367
Comment 35 EFL EWS Bot 2013-08-27 14:59:27 PDT 
Comment on attachment 209800 [details]
patch 5

Attachment 209800 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1621046
Comment 36 Geoffrey Garen 2013-08-27 15:02:14 PDT 
/mnt/eflews/webkit/WebKit/Source/WebCore/bindings/js/JSIDBDatabaseCustom.cpp: In member function 'JSC::JSValue WebCore::JSIDBDatabase::createObjectStore(JSC::ExecState*)':
/mnt/eflews/webkit/WebKit/Source/WebCore/bindings/js/JSIDBDatabaseCustom.cpp:49:68: error: 'throwError' was not declared in this scope
make[2]: *** [Source/WebCore/CMakeFiles/WebCore.dir/bindings/js/JSIDBDatabaseCustom.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
/mnt/eflews/webkit/WebKit/Source/WebCore/bindings/js/JSIDBObjectStoreCustom.cpp: In member function 'JSC::JSValue WebCore::JSIDBObjectStore::createIndex(JSC::ExecState*)':
/mnt/eflews/webkit/WebKit/Source/WebCore/bindings/js/JSIDBObjectStoreCustom.cpp:49:117: error: 'throwError' was not declared in this scope
/mnt/eflews/webkit/WebKit/Source/WebCore/bindings/js/JSIDBObjectStoreCustom.cpp:5Failed to run "['Tools/Scripts/build-webkit', '--release', '--efl', '--update-efl', '--no-webkit1', '--makeargs="-j8"']" exit_code: 2
Comment 37 Build Bot 2013-08-27 15:09:13 PDT 
Comment on attachment 209800 [details]
patch 5

Attachment 209800 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1618051

New failing tests:
fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html
Comment 38 Build Bot 2013-08-27 15:09:19 PDT 
Created attachment 209806 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.4
Comment 39 EFL EWS Bot 2013-08-27 15:23:36 PDT 
Comment on attachment 209800 [details]
patch 5

Attachment 209800 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1623051
Comment 40 Chris Curtis 2013-08-27 15:29:40 PDT 
Created attachment 209809 [details]
patch 6

Fixed the other 2 ports. The layout test will still fall.
Comment 41 Build Bot 2013-08-27 17:26:00 PDT 
Comment on attachment 209809 [details]
patch 6

Attachment 209809 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1621084

New failing tests:
fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html
Comment 42 Build Bot 2013-08-27 17:26:04 PDT 
Created attachment 209825 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.4
Comment 43 Build Bot 2013-08-27 19:02:56 PDT 
Comment on attachment 209809 [details]
patch 6

Attachment 209809 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1620099

New failing tests:
fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html
Comment 44 Build Bot 2013-08-27 19:03:02 PDT 
Created attachment 209835 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.4
Comment 45 Build Bot 2013-08-27 19:15:59 PDT 
Comment on attachment 209809 [details]
patch 6

Attachment 209809 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/1623106
Comment 46 Chris Curtis 2013-08-28 12:07:34 PDT 
Created attachment 209914 [details]
patch 7
Comment 47 Geoffrey Garen 2013-08-28 14:17:37 PDT 
Comment on attachment 209914 [details]
patch 7

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

I would normally mark this r+, and then let you resolve these comments before landing, but since you're using the commit queue, you'll need to upload a new patch to land.

> Source/JavaScriptCore/runtime/VM.cpp:595
> +        // No range information, so give a few characters of context

Let's upgrade this to a complete sentence by adding a "." at the end.

> Source/JavaScriptCore/runtime/VM.cpp:601
> +        // then strip whitespace.

Let's upgrade this to a complete sentence by capitalizing "Then".

> Source/JavaScriptCore/runtime/VM.cpp:637
> +        // FIXME: should only really be adding these properties to VM generated exceptions,

Let's upgrade this to a complete sentence by adding a "We".

> LayoutTests/fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html:94
> +            do{

Need a space after "do" here.

> LayoutTests/fast/xmlhttprequest/xmlhttprequest-recursive-sync-event.html:98
> +                    hasCompensatedAlready++ ;             
> +            } while (hasCompensatedAlready == 1 && lastReadyStateObserved == 1);

Why did hasCompensatedAlready change to a count when we're only trying to count from 0 to 1? That seems like a boolean to me. If it is a real count, we should rename hasCompensatedAlready to something like "compensationCount", to indicate that it's a count.
Comment 48 Chris Curtis 2013-08-28 16:00:38 PDT 
Created attachment 209927 [details]
patch 7 followup
Comment 49 Chris Curtis 2013-08-28 16:46:41 PDT 
Comment on attachment 209927 [details]
patch 7 followup

Uploading new patch soon
Comment 50 Chris Curtis 2013-08-28 16:55:03 PDT 
Created attachment 209935 [details]
patch 8
Comment 51 Geoffrey Garen 2013-08-28 16:58:46 PDT 
Comment on attachment 209935 [details]
patch 8

r=me
Comment 52 WebKit Commit Bot 2013-08-28 17:27:41 PDT 
Comment on attachment 209935 [details]
patch 8

Clearing flags on attachment: 209935

Committed r154797: <http://trac.webkit.org/changeset/154797>
Comment 53 WebKit Commit Bot 2013-08-28 17:27:47 PDT 
All reviewed patches have been landed.  Closing bug.