Add support for backend search in script content.
Created attachment 109156 [details] Patch
Attachment 109156 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/http..." exit_code: 1 Source/WebCore/inspector/InspectorDebuggerAgent.cpp:324: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.h:84: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 2 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 109156 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109156&action=review > LayoutTests/http/tests/inspector/search/search-in-concatenated-script.html:10 > + InspectorTest._runAfterResourcesAreCreated(["search-concatenated.html"], step2); You should wait for resource loading to finish. > LayoutTests/http/tests/inspector/search/search-in-script.html:10 > + InspectorTest._runAfterResourcesAreCreated(["search.js"], step2); ditto > Source/WebCore/inspector/InspectorDebuggerAgent.cpp:326 > + String content = m_scripts.get(scriptId).source; You should look it up first.
Created attachment 109301 [details] Patch
Attachment 109301 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/http..." exit_code: 1 Source/WebCore/inspector/InspectorDebuggerAgent.cpp:324: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.h:84: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 2 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 109301 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109301&action=review > Source/WebCore/ChangeLog:3 > + Web Inspector: Add support for backend search in script content. The title is a bit misleading since you have front-end pieces in this change as well. > Source/WebCore/inspector/InspectorDebuggerAgent.cpp:330 > + *results = ContentSearchUtils::searchInTextByLines(query, source); Searching in missing script should be an error. > Source/WebCore/inspector/InspectorDebuggerAgent.cpp:351 > + *scriptSource = source; ditto > Source/WebCore/inspector/front-end/Script.js:62 > + callback(searchMatches); Please log potential error using console.error
Committed r96491: <http://trac.webkit.org/changeset/96491>
Two test failed after r96491 The tests: http/tests/inspector/search/search-in-concatenated-script.html Diff: --- /ramdisk/qt-linux-32-release-qt470/build/layout-test-results/http/tests/inspector/search/search-in-concatenated-script-expected.txt 2011-10-03 06:25:47.786125385 -0700 +++ /ramdisk/qt-linux-32-release-qt470/build/layout-test-results/http/tests/inspector/search/search-in-concatenated-script-actual.txt 2011-10-03 06:25:47.786125385 -0700 @@ -2,14 +2,5 @@ Bug 69015 http://127.0.0.1:8000/inspector/search/resources/search-concatenated.html -Search matches: -lineNumber: 3, line: 'function searchTestUniqueString()' -lineNumber: 6, line: ' // searchTestUniqueString two occurences on the same line searchTestUniqueString' -lineNumber: 12, line: ' searchTestUniqueString();' -lineNumber: 13, line: ' // SEARCHTestUniqueString();' -lineNumber: 18, line: 'function searchTestUniqueString2()' -lineNumber: 21, line: ' /* searchTestUniqueString two occurences on the same line searchTestUniqueString */ } ' -lineNumber: 21, line: ' function doSomething2() { searchTestUniqueString();' -lineNumber: 22, line: ' // SEARCHTestUniqueString();' - +error: Exception during test execution: TypeError: 'undefined' is not an object http/tests/inspector/search/search-in-script.html Diff: --- /ramdisk/qt-linux-32-release-qt470/build/layout-test-results/http/tests/inspector/search/search-in-script-expected.txt 2011-10-03 06:25:48.538039183 -0700 +++ /ramdisk/qt-linux-32-release-qt470/build/layout-test-results/http/tests/inspector/search/search-in-script-actual.txt 2011-10-03 06:25:48.538039183 -0700 @@ -1,11 +1,5 @@ Tests script search in inspector debugger agent. Bug 69015 -http://127.0.0.1:8000/inspector/search/resources/search.js -Search matches: -lineNumber: 0, line: 'function searchTestUniqueString()' -lineNumber: 3, line: ' // searchTestUniqueString two occurences on the same line searchTestUniqueString' -lineNumber: 9, line: ' searchTestUniqueString();' -lineNumber: 10, line: ' // SEARCHTestUniqueString();' - +error: Exception during test execution: TypeError: 'undefined' is not an object
New bug report filed for Qt fails: https://bugs.webkit.org/show_bug.cgi?id=69265
This may have broken script-formatter.html on Snow Leopard: http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r96571%20(33608)/inspector/debugger/script-formatter-pretty-diff.html
Two tests added by this patch are failing on Snow Leopard: http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r96571%20(33608)/http/tests/inspector/search/search-in-script-pretty-diff.html http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r96571%20(33608)/http/tests/inspector/search/search-in-concatenated-script-pretty-diff.html
Created attachment 109605 [details] Patch
Attachment 109605 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/http..." exit_code: 1 Source/WebCore/inspector/InspectorDebuggerAgent.cpp:324: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.h:84: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Reopen the bug.
Comment on attachment 109605 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109605&action=review > Source/WebCore/inspector/front-end/ContentProviders.js:72 > + if (!this._sortedScriptsArray) { You should not cache this result. > Source/WebCore/inspector/front-end/Script.js:52 > + this._source = source || ""; You should be explicit about the error and do this._source = error ? "" : source;
Created attachment 109620 [details] Patch
Attachment 109620 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/http..." exit_code: 1 Source/WebCore/inspector/InspectorDebuggerAgent.cpp:324: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.h:84: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 109620 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109620&action=review > Source/WebCore/inspector/front-end/ContentProviders.js:71 > + { Please cache the result. > Source/WebCore/inspector/front-end/ContentProviders.js:84 > + if (lineNumber > scripts[i].lineOffset || (lineNumber === scripts[i].lineOffset && columnNumber > scripts[i].columnOffset)) I think this function could and should be 2 times shorter, just get rid of lineNumber and columnNumber variables and rewrite this check. > Source/WebCore/inspector/front-end/ContentProviders.js:119 > + if (!scriptsLeft) { Prefer early return.
Created attachment 109633 [details] Patch
Attachment 109633 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/http..." exit_code: 1 Source/WebCore/inspector/InspectorDebuggerAgent.cpp:324: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.h:84: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 109633 [details] Patch Attachment 109633 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9940401 New failing tests: inspector/debugger/debugger-activation-crash2.html
Comment on attachment 109633 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109633&action=review Looks good. > Source/WebCore/inspector/front-end/ContentProviders.js:72 > + if (!this._sortedScriptsArray) { Prefer early return. > Source/WebCore/inspector/front-end/ContentProviders.js:81 > + if (!scripts.length) RawSourceCode always has at least one script, you don't need this check. > Source/WebCore/inspector/front-end/ContentProviders.js:88 > + if (previousScript.endLine > scripts[i].lineOffset || (scripts[i - 1].endLine === scripts[i].lineOffset && columnNumber > scripts[i].columnOffset)) Consider negating this check.
Created attachment 109659 [details] Patch
Attachment 109659 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/http..." exit_code: 1 Source/WebCore/inspector/InspectorDebuggerAgent.cpp:324: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorDebuggerAgent.h:84: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 109659 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109659&action=review >> Source/WebCore/inspector/InspectorDebuggerAgent.cpp:324 >> +void InspectorDebuggerAgent::searchInContent(ErrorString* error, const String& scriptId, const String& query, RefPtr<InspectorArray>* results) > > The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] We don't use pointers to get values out of a function. Please use a reference instead. r- because of this.
> We don't use pointers to get values out of a function. Please use a reference instead. r- because of this. That's the way communication between frontend and backend is implemented in Inspector.
Comment on attachment 109659 [details] Patch (In reply to comment #26) > > We don't use pointers to get values out of a function. Please use a reference instead. r- because of this. > > That's the way communication between frontend and backend is implemented in Inspector. That seems wrong. Why can't we use refenrece?
(In reply to comment #27) > (From update of attachment 109659 [details]) > (In reply to comment #26) > > > We don't use pointers to get values out of a function. Please use a reference instead. r- because of this. > > > > That's the way communication between frontend and backend is implemented in Inspector. > > That seems wrong. Why can't we use refenrece? Inspector*Agents are called by InspectorBackendDispatcher which is generated automatically. This generator was written long ago and could be fixed but this is definitely out of scope of this change. I filed https://bugs.webkit.org/show_bug.cgi?id=69366 for that.
(In reply to comment #28) > Inspector*Agents are called by InspectorBackendDispatcher which is generated automatically. > This generator was written long ago and could be fixed but this is definitely out of scope of this change. > I filed https://bugs.webkit.org/show_bug.cgi?id=69366 for that. Great. Thanks.
Comment on attachment 109659 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109659&action=review Looks good. > Source/WebCore/inspector/front-end/ContentProviders.js:80 > + var scriptOpenTagLength = "<script>".length; Please add _scriptOpenTag static field to ConcatenatedScriptsContentProvider class and replace scriptOpenTagLength with scriptOpenTag.length.
Committed r96703: <http://trac.webkit.org/changeset/96703>