Summary: | [Chromium] Upstream the rest of DevTools WebKit API implementation | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yury Semikhatsky <yurys> | ||||||||
Component: | WebKit API | Assignee: | Yury Semikhatsky <yurys> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | dglazkov, eric, pfeldman, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Yury Semikhatsky
2010-01-29 04:52:48 PST
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. |