[webkit-reviews] review canceled: [Bug 40422] Web Inspector: Port performSearch from InjectedScript to InspectorDOMAgent. : [Attachment 58449] [PATCH] More imports! (is this going to end?).

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jun 11 01:00:48 PDT 2010


Yury Semikhatsky <yurys at chromium.org> has canceled Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 40422: Web Inspector: Port performSearch from InjectedScript to
InspectorDOMAgent.
https://bugs.webkit.org/show_bug.cgi?id=40422

Attachment 58449: [PATCH] More imports! (is this going to end?).
https://bugs.webkit.org/attachment.cgi?id=58449&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>

WebCore/inspector/InspectorResource.cpp: 
 +	    jsonObject.set("suggestedFilename", m_suggestedFilename);
Remove suggestedFilename handling from inspecor.js too.

WebCore/inspector/InspectorDOMAgent.cpp:1554
 +	m_pendingMatchJobs.removeFirst();
Use takeFirst instead.

WebCore/inspector/InspectorDOMAgent.cpp:644
 +	RefPtr<NodeList> frameOwners =
mainFrameDocument()->querySelectorAll("iframe, frame, object", ec);
Why not use FrameTree to iterate all frames in the page?

WebCore/inspector/InspectorDOMAgent.cpp:696
 +	    m_pendingMatchJobs.append(new MatchPlainTextJob(*it,
escapedQuery));
Please put this line first in the loop body and remove it from all the ifs
above.

WebCore/inspector/InspectorDOMAgent.cpp:674
 +		m_pendingMatchJobs.append(new MatchXPathJob(*it,
"//*[contains(name(), '" + escapedTagNameQuery + "')]"));
This could be implemented as

//*[contains(name(), 'escapedTagNameQuery') and
string-length(substring-after(name(), 'escapedTagNameQuery')) = 0]


WebCore/inspector/InspectorDOMAgent.h:78
 +	class MatchJob {
Can you leave only forward declaration of the class here and put its definition
into InspectorDOMAgent.cpp?


More information about the webkit-reviews mailing list