The goal is to enable "JAVASCRIPT_DEBUGGER" in Chromium port and have profiler / debugger stuff compiled --- this requires not to depend directly on JSC, for example. As a first step, I'm removing custom implementations of Console, and migrating InspectorController towards using 'Script...' types instead of 'JSC::' ones. Temporarily, there will be stuff under "#if ENABLE(JAVASCRIPT_DEBUGGER) || USE(V8)" conditionals. The second part will be removed back after migration will have been finished, and thus JAVASCRIPT_DEBUGGER will be enabled in Chromium.
Created attachment 47935 [details] proposed patch
Attachment 47935 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/page/Console.h:42: Code inside a namespace should not be indented. [whitespace/indent] [4] Skipping input 'WebCore/bindings/v8/custom/V8ConsoleCustom.cpp': Can't open for reading WebCore/bindings/js/ScriptProfiler.cpp:31: You should add a blank line after implementation file's own header. [build/include_order] [4] WebCore/bindings/v8/ScriptProfile.h:40: Place brace on its own line for function definitions. [whitespace/braces] [4] WebCore/bindings/v8/ScriptProfiler.cpp:32: You should add a blank line after implementation file's own header. [build/include_order] [4] WebCore/bindings/v8/ScriptProfiler.cpp:40: context_scope is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 5 If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 47941 [details] proposed patch - fixed style violations
Comment on attachment 47941 [details] proposed patch - fixed style violations r- for no comments on the migration scenario. > + > + const String& title() const { return m_title; } Return copy in order not to deal with the ownership. > - JSC::StringBuilder message; > - message.append("Profile \"webkit-profile://"); > - message.append((UString)encodeWithURLEscapeSequences(CPUProfileType)); > - message.append("/"); > - message.append((UString)encodeWithURLEscapeSequences(profile->title())); > - message.append("#"); > - message.append(UString::from(profile->uid())); > - message.append("\" finished."); > - addMessageToConsole(JSMessageSource, LogMessageType, LogMessageLevel, message.release(), lineNumber, sourceURL); > + String message; > + message += "Profile \"webkit-profile://"; > + message += encodeWithURLEscapeSequences(CPUProfileType); > + message += "/"; > + message += encodeWithURLEscapeSequences(profile->title()); > + message += "#"; > + message += String::number(profile->uid()); > + message += "\" finished."; > + addMessageToConsole(JSMessageSource, LogMessageType, LogMessageLevel, message, lineNumber, sourceURL); > } > Why not to use String::format? > -void InspectorController::addStartProfilingMessageToConsole(const UString& title, unsigned lineNumber, const UString& sourceURL) > +void InspectorController::addStartProfilingMessageToConsole(const String& title, unsigned lineNumber, const String& sourceURL) > { > - JSC::StringBuilder message; > - message.append("Profile \"webkit-profile://"); > - message.append(encodeWithURLEscapeSequences(CPUProfileType)); > - message.append("/"); > - message.append(encodeWithURLEscapeSequences(title)); > - message.append("#0\" started."); > - addMessageToConsole(JSMessageSource, LogMessageType, LogMessageLevel, message.release(), lineNumber, sourceURL); > + String message; > + message += "Profile \"webkit-profile://"; > + message += encodeWithURLEscapeSequences(CPUProfileType); > + message += "/"; > + message += encodeWithURLEscapeSequences(title); > + message += "#0\" started."; > + addMessageToConsole(JSMessageSource, LogMessageType, LogMessageLevel, message, lineNumber, sourceURL); > } > ditto > +#if ENABLE(JAVASCRIPT_DEBUGGER) || USE(V8) > > -#if ENABLE(INSPECTOR) > +#if ENABLE(INSPECTOR) && USE(JSC) > InspectorController* controller = page->inspectorController(); > // FIXME: log a console message when profiling is disabled. > -#if ENABLE(INSPECTOR) > +#if ENABLE(INSPECTOR) && USE(JSC) > resolvedTitle = controller->getCurrentUserInitiatedProfileName(true); > #else Could you comment on the fact that this is a temporary measure that will exist until JAVASCRIPT_DEBUGGER is enabled in V8 case?
Created attachment 48003 [details] comments addressed, fixed style violations found by an updated version of style checking script There can be an error from style checker concerning absence of `#include "config.h"' in JSBindingsAllInOne.cpp. I'm considering this as a problem in the style checker, not my fault.
Comment on attachment 48003 [details] comments addressed, fixed style violations found by an updated version of style checking script Clearing flags on attachment: 48003 Committed r54277: <http://trac.webkit.org/changeset/54277>
All reviewed patches have been landed. Closing bug.