There are some parts of DevTools WebKit API implementation that are not specific to Chromium architecture but still live in the Chromium repository. We should migrate them to WebKit.
Created attachment 47707 [details] patch Add API methods for loading injected script and dispatch script sources from resource bundle.
Attachment 47707 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKit/chromium/public/WebDevToolsAgentClient.h:35: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 47707 [details] patch Please fix style.
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebKit/chromium/ChangeLog M WebKit/chromium/public/WebDevToolsAgentClient.h Committed r54057
(In reply to comment #3) > (From update of attachment 47707 [details]) > Please fix style. Done.
Created attachment 47842 [details] patch
Attachment 47842 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 Last 3072 characters of output: h:60: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/ToolsAgent.h:61: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/ToolsAgent.h:63: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/ToolsAgent.h:64: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/ToolsAgent.h:65: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/WebDevToolsAgentImpl.h:43: Code inside a namespace should not be indented. [whitespace/indent] [4] WebKit/chromium/src/WebDevToolsFrontendImpl.h:46: Code inside a namespace should not be indented. [whitespace/indent] [4] WebKit/chromium/src/ProfilerAgentImpl.cpp:38: Place brace on its own line for function definitions. [whitespace/braces] [4] WebKit/chromium/src/ProfilerAgentImpl.cpp:39: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/ProfilerAgentImpl.cpp:42: Place brace on its own line for function definitions. [whitespace/braces] [4] WebKit/chromium/src/ProfilerAgentImpl.cpp:43: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/ProfilerAgentImpl.cpp:44: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/ProfilerAgentImpl.cpp:45: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/ProfilerAgentImpl.cpp:46: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/ProfilerAgentImpl.cpp:47: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/DebuggerAgentManager.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] WebKit/chromium/src/DebuggerAgent.h:39: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/DebuggerAgent.h:40: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/DebuggerAgent.h:45: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/DebuggerAgent.h:46: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/DebuggerAgent.h:47: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/DebuggerAgent.h:48: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/src/DevToolsRPC.h:86: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 76 If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 47842 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/226008
Comment on attachment 47842 [details] patch Bunch of style nits, other than that r+. > + 'src/DebuggerAgent.h', > + 'src/DebuggerAgentImpl.cpp', > + 'src/DebuggerAgentImpl.h', > + 'src/DebuggerAgentManager.cpp', > + 'src/DebuggerAgentManager.h', > + 'src/DevToolsRPC.h', > + 'src/DevToolsRPCJS.h', Indent. > + > +#define APU_AGENT_DELEGATE_STRUCT(METHOD0, METHOD1, METHOD2, METHOD3, MEHTOD4, METHOD5) \ > + /* Sends a json object to apu. */ \ > + METHOD1(dispatchToApu, String /* data */) > + 4 space indent? > + > + v8::Handle<v8::Value> args[] = { > + functionNameWrapper, > + jsonArgsWrapper, > + callIdWrapper > + }; 4 space indent? > + v8::Handle<v8::Value> args[] = { > + v8::Local<v8::Value>() Indent. > +WebCore::Page* DebuggerAgentImpl::page() > +{ > + return m_webViewImpl->page(); > +} Indent. > + > +#include "webkit/glue/glue_util.h" > +#include "webkit/glue/webkit_glue.h" > + Does this compile? > + virtual void dispatchOnInjectedScript( > + int callId, > + int injectedScriptId, > + const WebCore::String& functionName, > + const WebCore::String& jsonArgs, > + bool async); Use single line here in and cpp. > +WebDevToolsFrontend* WebDevToolsFrontend::create( > + WebView* view, > + WebDevToolsFrontendClient* client, > + const WebString& applicationLocale) > +{ > + return new WebDevToolsFrontendImpl( > + static_cast<WebViewImpl*>(view), > + client, > + applicationLocale); > +} Why wrapping?
Created attachment 47911 [details] patch All the comments have been addressed. Style errors fixed.
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebKit/chromium/ChangeLog M WebKit/chromium/WebKit.gyp A WebKit/chromium/src/APUAgentDelegate.h A WebKit/chromium/src/BoundObject.cpp A WebKit/chromium/src/BoundObject.h A WebKit/chromium/src/DebuggerAgent.h A WebKit/chromium/src/DebuggerAgentImpl.cpp A WebKit/chromium/src/DebuggerAgentImpl.h A WebKit/chromium/src/DebuggerAgentManager.cpp A WebKit/chromium/src/DebuggerAgentManager.h A WebKit/chromium/src/DevToolsRPC.h A WebKit/chromium/src/DevToolsRPCJS.h A WebKit/chromium/src/ProfilerAgent.h A WebKit/chromium/src/ProfilerAgentImpl.cpp A WebKit/chromium/src/ProfilerAgentImpl.h A WebKit/chromium/src/ToolsAgent.h A WebKit/chromium/src/WebDevToolsAgentImpl.cpp A WebKit/chromium/src/WebDevToolsAgentImpl.h A WebKit/chromium/src/WebDevToolsFrontendImpl.cpp A WebKit/chromium/src/WebDevToolsFrontendImpl.h Committed r54232
Looks like this was landed. Closing bug.
Consider trying "webkit-patch" for landing your bugs. That tool will automatically update/close bugs for you. See "webkit-patch --help"
(In reply to comment #13) > Consider trying "webkit-patch" for landing your bugs. That tool will > automatically update/close bugs for you. > > See "webkit-patch --help" Thaks for the suggestion, will try. But this particular bug was not closed intentionally since there are some pieces of devtools implemented in JavaScript that are to be upstreamed. It might be a good idea to file a separate bug for them though.
One bug per change is generally preferred, yes.