[Webkit-unassigned] [Bug 48077] HTMLParserScheduler should be suspended when page loading is deferred

[bugzilla-daemon at webkit.org]

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





--- Comment #22 from Yong Li <yong.li.webkit at gmail.com>  2010-11-02 08:49:09 PST ---
(In reply to comment #14)
> (From update of attachment 71896 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=71896&action=review
> 
> Something about this change doesn't seem very elegant.  I think the problem is we're plumbing this even though the DocumentParser interface even though it's only relevant for an HTMLDocumentParser.  Maybe Eric has some thoughts.  R- for nits below.
> 
> > WebCore/dom/DocumentParser.h:95
> > +    virtual void willDeferLoading() {}
> > +    virtual void didResumeLoading() {}
> 
> Please don't put implementations of virtual methods in the header.  That leads to binary bloat.  What about all the other implementations of this interface?  Do they not care about these events?
> 

I also prefer to keep virtual methods' implementations in cpp file. But there are tons of virtual methods implemented in webkit header files. I'm not sure which is more webkit style. Other DocumentParser don't have this problem yet.

> > WebCore/html/parser/HTMLParserScheduler.h:95
> > +    void suspend()
> > +    {
> > +        ASSERT(!m_isSuspendedWithActiveTimer);
> > +        if (!m_continueNextChunkTimer.isActive())
> > +            return;
> > +        m_isSuspendedWithActiveTimer = true;
> > +        m_continueNextChunkTimer.stop();
> > +    }
> > +
> > +    void resume()
> > +    {
> > +        ASSERT(!m_continueNextChunkTimer.isActive());
> > +        if (!m_isSuspendedWithActiveTimer)
> > +            return;
> > +        m_isSuspendedWithActiveTimer = false;
> > +        m_continueNextChunkTimer.startOneShot(0);
> > +    }
> 
> These should go in the cpp file.
> 

Totally agree. But almost all other methods of the scheduler are implemented in the header file including those containing many lines. I think they should be kept in consistent style. Should I move some of them to the cpp file?

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