Bug 43900 - Web Inspector: allow interrupting long running JS to execute inspector command
Summary: Web Inspector: allow interrupting long running JS to execute inspector command
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yury Semikhatsky
URL:
Keywords:
Depends on:
Blocks: 41706
  Show dependency tree / graph
 
Reported: 2010-08-12 01:50 PDT by Yury Semikhatsky
Modified: 2010-09-07 01:20 PDT (History)
12 users (show)

See Also:

Attachments
Patch (11.37 KB, patch)
2010-08-12 03:11 PDT, Yury Semikhatsky 		no flags Details | Formatted Diff | Diff
Patch (18.76 KB, patch)
2010-08-12 07:21 PDT, Yury Semikhatsky 		no flags Details | Formatted Diff | Diff
Patch (20.72 KB, patch)
2010-08-12 07:38 PDT, Yury Semikhatsky 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2010-08-12 01:50:14 PDT 
Web Inspector: allow interrupting long running JS to execute inspector command
Comment 1 Yury Semikhatsky 2010-08-12 03:11:22 PDT 
Created attachment 64201 [details]
Patch
Comment 2 Ilya Tikhonovsky 2010-08-12 03:22:17 PDT 
Comment on attachment 64201 [details]
Patch

WebKit/chromium/src/WebDevToolsAgentImpl.cpp:479
 +  bool WebDevToolsAgent::shouldInterruptForMessage(const WebString& message)
It would be better to move parsing part of the method to InspectorBackendDispatcher.
Comment 3 Yury Semikhatsky 2010-08-12 03:35:04 PDT 
(In reply to comment #2)
> (From update of attachment 64201 [details])
> WebKit/chromium/src/WebDevToolsAgentImpl.cpp:479
>  +  bool WebDevToolsAgent::shouldInterruptForMessage(const WebString& message)
> It would be better to move parsing part of the method to InspectorBackendDispatcher.

Will do. Also I'm going to generate constant declarations for the commands on InspectorBackendDispatcher.h
Comment 4 Pavel Feldman 2010-08-12 03:52:06 PDT 
Comment on attachment 64201 [details]
Patch

Rename the web* methods please.
Comment 5 Yury Semikhatsky 2010-08-12 07:21:46 PDT 
Created attachment 64219 [details]
Patch
Comment 6 Ilya Tikhonovsky 2010-08-12 07:29:39 PDT 
Comment on attachment 64219 [details]
Patch

WebCore/inspector/CodeGeneratorInspector.pm:312
 +      push(@function, "    const char* backendFunctionName = ${functionName}Cmd;");
it would be better to use ${functionName}Cmd directly

WebCore/inspector/CodeGeneratorInspector.pm:462
 +      if (array->length() < 1)
!array->length()
Comment 7 WebKit Review Bot 2010-08-12 07:30:06 PDT 
Attachment 64219 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/3729090
Comment 8 Yury Semikhatsky 2010-08-12 07:38:15 PDT 
Created attachment 64223 [details]
Patch
Comment 9 Yury Semikhatsky 2010-08-12 07:38:57 PDT 
(In reply to comment #6)
> (From update of attachment 64219 [details])
> WebCore/inspector/CodeGeneratorInspector.pm:312
>  +      push(@function, "    const char* backendFunctionName = ${functionName}Cmd;");
> it would be better to use ${functionName}Cmd directly
> 
Done.

> WebCore/inspector/CodeGeneratorInspector.pm:462
>  +      if (array->length() < 1)
> !array->length()
Done.
Comment 10 WebKit Review Bot 2010-08-12 09:13:01 PDT 
Attachment 64223 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/3723084
Comment 11 Pavel Feldman 2010-08-12 22:41:55 PDT 
Comment on attachment 64223 [details]
Patch

This breaks chromium, other than that it is Ok.
Comment 12 Pavel Feldman 2010-08-12 22:46:39 PDT 
Comment on attachment 64223 [details]
Patch

r- for not compiling on chromium.
Comment 13 Yury Semikhatsky 2010-08-12 23:31:14 PDT 
(In reply to comment #12)
> (From update of attachment 64223 [details])
> r- for not compiling on chromium.
It successfully compiled on Chromium checkout on my linux box. It might be a bot issue.
Comment 14 Pavel Feldman 2010-08-12 23:32:32 PDT 
Comment on attachment 64223 [details]
Patch

r+ if you are saying it is a bot issue.
Comment 15 Yury Semikhatsky 2010-08-13 00:08:53 PDT 
Committed r65304