We already have a way for a Page's console to log to the system console. We should provide a way for this to work with JSContext's console. An application user default seems reasonable. We could also make API/SPI to toggle at runtime.
<rdar://problem/16488820>
Created attachment 228304 [details] [PATCH] Proposed Fix cq- because I haven't tested with iOS yet and I'm not sure if printf will work. It works on OS X though, so getting the general idea reviewed.
Attachment 228304 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/inspector/JSConsoleClient.cpp:73: Missing space before { [whitespace/braces] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 228304 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=228304&action=review > Source/JavaScriptCore/inspector/JSConsoleClient.cpp:62 > + Boolean preference = CFPreferencesGetAppBooleanValue(CFSTR("JavaScriptCoreConsoleLogToSystemConsole"), kCFPreferencesCurrentApplication, &keyExistsAndHasValidFormat); JavaScriptCoreConsoleLogToSystemConsole implies only console.log is printed. A verb like "output" would help too. Maybe: OutputJavaScriptCoreConsoleToSystemConsole. Does this work for WebKit too? Maybe OutputJavaScriptConsoleToSystemConsole if so. > Source/JavaScriptCore/inspector/JSConsoleClient.cpp:82 > + if (JSConsoleClient::logToSystemConsole()) No need for JSConsoleClient:: right?
(In reply to comment #4) > (From update of attachment 228304 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=228304&action=review > > > Source/JavaScriptCore/inspector/JSConsoleClient.cpp:62 > > + Boolean preference = CFPreferencesGetAppBooleanValue(CFSTR("JavaScriptCoreConsoleLogToSystemConsole"), kCFPreferencesCurrentApplication, &keyExistsAndHasValidFormat); > > JavaScriptCoreConsoleLogToSystemConsole implies only console.log is printed. A verb like "output" would help too. Maybe: OutputJavaScriptCoreConsoleToSystemConsole. Does this work for WebKit too? Maybe OutputJavaScriptConsoleToSystemConsole if so. I want the preference to start with "JavaScriptCore" as a namespacing thing. There is one other CFPreference in JavaScriptCore and it does the same. Any preference among these? - "JavaScriptCoreOutputConsoleMessagesToSystemConsole" - "JavaScriptCoreLogConsoleMessagesToSystemConsole" - "JavaScriptCoreConsoleLogToSystemConsoleEnabled" > > Source/JavaScriptCore/inspector/JSConsoleClient.cpp:82 > > + if (JSConsoleClient::logToSystemConsole()) > > No need for JSConsoleClient:: right? Correct, but I prefer the explicit prefix because it makes it obvious that this is a static/class function and not a member function. If we have a style one way or the other I can match.
Comment on attachment 228304 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=228304&action=review >>> Source/JavaScriptCore/inspector/JSConsoleClient.cpp:62 >>> + Boolean preference = CFPreferencesGetAppBooleanValue(CFSTR("JavaScriptCoreConsoleLogToSystemConsole"), kCFPreferencesCurrentApplication, &keyExistsAndHasValidFormat); >> >> JavaScriptCoreConsoleLogToSystemConsole implies only console.log is printed. A verb like "output" would help too. Maybe: OutputJavaScriptCoreConsoleToSystemConsole. Does this work for WebKit too? Maybe OutputJavaScriptConsoleToSystemConsole if so. > > I want the preference to start with "JavaScriptCore" as a namespacing thing. There is one other CFPreference in JavaScriptCore and it does the same. > > Any preference among these? > > - "JavaScriptCoreOutputConsoleMessagesToSystemConsole" > - "JavaScriptCoreLogConsoleMessagesToSystemConsole" > - "JavaScriptCoreConsoleLogToSystemConsoleEnabled" Prefixing with JSC makes sense. I like JavaScriptCoreOutputConsoleMessagesToSystemConsole. >>> Source/JavaScriptCore/inspector/JSConsoleClient.cpp:82 >>> + if (JSConsoleClient::logToSystemConsole()) >> >> No need for JSConsoleClient:: right? > > Correct, but I prefer the explicit prefix because it makes it obvious that this is a static/class function and not a member function. If we have a style one way or the other I can match. It looks like you are calling a base class method with this syntax. I know I have seen other static methods called without a prefix in WebCore. Not sure there is a formal style guide.
Created attachment 228546 [details] [PATCH] Revised This refactors all the printfs instead StringBuilder + WTFLogAlways. It also includes the updated default name. I tested on iOS this works (the printf would have too, but I think this is nicer).
Attachment 228546 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/inspector/JSConsoleClient.cpp:73: Missing space before { [whitespace/braces] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 228546 [details] [PATCH] Revised View in context: https://bugs.webkit.org/attachment.cgi?id=228546&action=review > Source/JavaScriptCore/inspector/JSConsoleClient.cpp:75 > + static std::once_flag initializeLogging; > + std::call_once(initializeLogging, []{ > + JSConsoleClient::initializeLogToSystemConsole(); > + }); This makes the default behave differently than the WebKit2 equivalent setting. It can only be changed between executions, not while the app is running. That seems fine, but it might be limiting if an app needed to change it at runtime (like with a command line flag or checkbox.) The solution would require always calling CFPreferencesGetAppBooleanValue, or exposing a JSC API to change it at runtime. We can cross that bridge when needed.
Comment on attachment 228546 [details] [PATCH] Revised Clearing flags on attachment: 228546 Committed r166799: <http://trac.webkit.org/changeset/166799>
All reviewed patches have been landed. Closing bug.