Bug 119548 – Refactoring of throw exception.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Chris Curtis]

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.

[Chris Curtis]

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.

[WebKit Commit Bot]

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.

[Early Warning System Bot]

Comment on attachment 208271 [details]
patch

Attachment 208271 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1376303

[Early Warning System Bot]

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

[Geoffrey Garen]

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

[Geoffrey Garen]

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  <[e-mail]>
> +013-08-06  Filip Pizlo  <[e-mail]>

Please revert.

[Chris Curtis]

(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.

[Chris Curtis]

Created attachment 208943 [details]
patch2

[Darin Adler]

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".

[Geoffrey Garen]

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(...);

[Chris Curtis]

Created attachment 209237 [details]
patch3

[Early Warning System Bot]

Comment on attachment 209237 [details]
patch3

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

[Early Warning System Bot]

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

[EFL EWS Bot]

Comment on attachment 209237 [details]
patch3

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

[EFL EWS Bot]

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

[Geoffrey Garen]

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'

[kov's GTK+ EWS bot]

Comment on attachment 209237 [details]
patch3

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

[Build Bot]

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

[Build Bot]

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

[Geoffrey Garen]

Comment on attachment 209237 [details]
patch3

Looks like some tests are failing, too.

[Chris Curtis]

Created attachment 209695 [details]
patch3

Not sure I got all the #includes on the other ports

[EFL EWS Bot]

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

[Early Warning System Bot]

Comment on attachment 209695 [details]
patch3

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

[Early Warning System Bot]

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

[EFL EWS Bot]

Comment on attachment 209695 [details]
patch3

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

[kov's GTK+ EWS bot]

Comment on attachment 209695 [details]
patch3

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

[Build Bot]

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

[Build Bot]

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

[Chris Curtis]

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

[WebKit Commit Bot]

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.

[Early Warning System Bot]

Comment on attachment 209795 [details]
patch 4

Attachment 209795 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1624031

[Early Warning System Bot]

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

[Chris Curtis]

Created attachment 209800 [details]
patch 5

more work on the qt port.

[kov's GTK+ EWS bot]

Comment on attachment 209800 [details]
patch 5

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

[EFL EWS Bot]

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

[Geoffrey Garen]

/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

[Build Bot]

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

[Build Bot]

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

[EFL EWS Bot]

Comment on attachment 209800 [details]
patch 5

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

[Chris Curtis]

Created attachment 209809 [details]
patch 6

Fixed the other 2 ports. The layout test will still fall.

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

Comment on attachment 209809 [details]
patch 6

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

[Chris Curtis]

Created attachment 209914 [details]
patch 7

[Geoffrey Garen]

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.

[Chris Curtis]

Created attachment 209927 [details]
patch 7 followup

[Chris Curtis]

Comment on attachment 209927 [details]
patch 7 followup

Uploading new patch soon

[Chris Curtis]

Created attachment 209935 [details]
patch 8

[Geoffrey Garen]

Comment on attachment 209935 [details]
patch 8

r=me

[WebKit Commit Bot]

Comment on attachment 209935 [details]
patch 8

Clearing flags on attachment: 209935

Committed r154797: <http://trac.webkit.org/changeset/154797>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.