Bug 40289 - Web Inspector: v8 debugger should always be enabled when frontend is attached.
Summary: Web Inspector: v8 debugger should always be enabled when frontend is attached.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Pavel Podivilov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-08 02:51 PDT by Pavel Podivilov
Modified: 2010-06-09 08:27 PDT (History)
4 users (show)

See Also:


Attachments
Proposed patch. (8.84 KB, patch)
2010-06-08 03:15 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Proposed patch. (8.84 KB, patch)
2010-06-08 03:30 PDT, Pavel Podivilov
yurys: review-
Details | Formatted Diff | Diff
Proposed patch. (9.38 KB, patch)
2010-06-09 03:20 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Proposed patch. (9.38 KB, patch)
2010-06-09 03:30 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Proposed patch. (deleted)
2010-06-09 03:35 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Proposed patch. (9.49 KB, patch)
2010-06-09 07:24 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Proposed patch. (9.50 KB, patch)
2010-06-09 07:35 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Podivilov 2010-06-08 02:51:35 PDT
Web Inspector: v8 debugger should always be enabled when frontend is attached.
Comment 1 Pavel Podivilov 2010-06-08 03:15:13 PDT
Created attachment 58127 [details]
Proposed patch.
Comment 2 WebKit Review Bot 2010-06-08 03:16:21 PDT
Attachment 58127 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKit/chromium/ChangeLog:5:  One or more unexpected \r (^M) found; better to use only a \n  [whitespace/carriage_return] [1]
Suppressing further [whitespace/carriage_return] reports for this file.
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Pavel Podivilov 2010-06-08 03:30:37 PDT
Created attachment 58128 [details]
Proposed patch.
Comment 4 Yury Semikhatsky 2010-06-09 02:06:44 PDT
Comment on attachment 58128 [details]
Proposed patch.

WebKit/chromium/src/WebDevToolsAgentImpl.cpp:465
 +      WebCString debuggerScriptJs = m_client->debuggerScriptSource();
Please surround it with #if ENABLE(V8_SCRIPT_DEBUG_SERVER) guard, otherwise looks good.
Comment 5 Yury Semikhatsky 2010-06-09 02:13:22 PDT
Comment on attachment 58128 [details]
Proposed patch.

DebuggerScript.js should be excluded from devtools.html script list. r- for that
Comment 6 Pavel Podivilov 2010-06-09 03:20:18 PDT
Created attachment 58230 [details]
Proposed patch.
Comment 7 WebKit Review Bot 2010-06-09 03:24:29 PDT
Attachment 58230 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/ChangeLog:2:  One or more unexpected \r (^M) found; better to use only a \n  [whitespace/carriage_return] [1]
Suppressing further [whitespace/carriage_return] reports for this file.
WebKit/chromium/ChangeLog:3:  One or more unexpected \r (^M) found; better to use only a \n  [whitespace/carriage_return] [1]
Suppressing further [whitespace/carriage_return] reports for this file.
Total errors found: 11 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Pavel Podivilov 2010-06-09 03:30:22 PDT
Created attachment 58231 [details]
Proposed patch.
Comment 9 WebKit Review Bot 2010-06-09 03:31:48 PDT
Attachment 58231 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/ChangeLog:2:  One or more unexpected \r (^M) found; better to use only a \n  [whitespace/carriage_return] [1]
Suppressing further [whitespace/carriage_return] reports for this file.
WebKit/chromium/ChangeLog:3:  One or more unexpected \r (^M) found; better to use only a \n  [whitespace/carriage_return] [1]
Suppressing further [whitespace/carriage_return] reports for this file.
Total errors found: 11 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Pavel Podivilov 2010-06-09 03:35:21 PDT
Created attachment 58232 [details]
Proposed patch.
Comment 11 Yury Semikhatsky 2010-06-09 04:48:34 PDT
Comment on attachment 58232 [details]
Proposed patch.

WebCore/inspector/InspectorController.cpp:439
 +          enableDebuggerFromFrontend(false);
This will cause all parsed scripts to be pushed to the frontend along with their content. It may be quite expensive and currently we don't send the sources until ScriptsPanel is shown. We should come up with something smarter here.
Comment 12 Yury Semikhatsky 2010-06-09 06:17:40 PDT
(In reply to comment #11)
> (From update of attachment 58232 [details])
> WebCore/inspector/InspectorController.cpp:439
>  +          enableDebuggerFromFrontend(false);
> This will cause all parsed scripts to be pushed to the frontend along with their content. It may be quite expensive and currently we don't send the sources until ScriptsPanel is shown. We should come up with something smarter here.

Please file a bug on this and put it as a FIXME at that line. We'll fix this in a separate change.
Comment 13 Pavel Podivilov 2010-06-09 07:24:30 PDT
Created attachment 58243 [details]
Proposed patch.
Comment 14 WebKit Review Bot 2010-06-09 07:28:11 PDT
Attachment 58243 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/inspector/InspectorController.cpp:442:  An else should appear on the same line as the preceding }  [whitespace/newline] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Pavel Podivilov 2010-06-09 07:35:36 PDT
Created attachment 58244 [details]
Proposed patch.
Comment 16 Yury Semikhatsky 2010-06-09 08:00:54 PDT
Comment on attachment 58244 [details]
Proposed patch.

Clearing flags on attachment: 58244

Committed r60893: <http://trac.webkit.org/changeset/60893>
Comment 17 Yury Semikhatsky 2010-06-09 08:01:06 PDT
All reviewed patches have been landed.  Closing bug.
Comment 18 WebKit Review Bot 2010-06-09 08:27:08 PDT
http://trac.webkit.org/changeset/60893 might have broken GTK Linux 64-bit Debug