[Webkit-unassigned] [Bug 110524] Add a way to watch pages for elements matching a CSS selector

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=110524





--- Comment #5 from Tony Chang <tony at chromium.org>  2013-02-25 13:13:08 PST ---
(From update of attachment 189861)
View in context: https://bugs.webkit.org/attachment.cgi?id=189861&action=review

> Source/Platform/chromium/public/WebVector.h:115
> +    void resize(size_t newSize)

I don't see any callers of this, so I assume it's used on the Chromium side.

> Source/Platform/chromium/public/WebVector.h:118
> +        for (size_t i = 0, e = std::min(m_size, newSize); i < e; ++i)

Nit: Please use a longer variable name than |e|.  Maybe sizeToCopy?

> Source/WebCore/css/StyleResolver.cpp:3401
> +                state.style()->removeCallbackSelector(
> +                    rule->selectorList().selectorsText());

It might be possible to move removeCallbackSelector/addCallbackSelector to ElementRareData.  Instead of hooking a destructor, you could hook RenderObject::styleDidChange to watch for style removal.  You would still need to add a bit to StyleRareNonInheritedData.cpp, but it would just have a simple setter/getter.  It would also avoid having to add a document pointer.

> Source/WebCore/dom/Document.cpp:2011
> +        OwnPtr<pair<HashSet<String>, HashSet<String> > > callbackSelectorChange =

Maybe use a typedef?

> Source/WebCore/dom/Document.h:1190
> +    void addSelectorMatch(String);
> +    void removeSelectorMatch(String);

Nit: I would name these params to make it clear what the parameter is.

> Source/WebKit/chromium/src/FrameLoaderClientImpl.cpp:1242
> +    if (m_webFrame->client()) {

I'm not sure if you need to check client() for NULL here.  Maybe someone with more knowledge of FrameLoader can comment.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.