[Webkit-unassigned] [Bug 88271] initial rendering microbenchmark

[bugzilla-daemon at webkit.org]

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





--- Comment #16 from Daniel Murphy <dmurph at chromium.org>  2012-06-26 12:14:00 PST ---
(In reply to comment #14)
> > Source/WebKit/chromium/public/WebView.h:459
> > +    virtual WebRenderingBenchmarkController* createRenderingBenchmarkController()
> 
> Caller assumes ownership? Why?
> 
> I think WebViewImpl should ownptr<renderingBenchmarkConttroller> (lazily create it) and this can just be an accessor.

That's fine as long as the controller is stateless.  Otherwise, it needs to be a factory.  Do you want to impose the stateless constraint on the controller, or the ownership constraint on the WebView?

> 
> > Source/WebKit/chromium/src/WebViewImplRenderingBenchmarkController.cpp:50
> > +    m_webViewImpl->layout();
> 
> Why expose layout if you're automatically laying out? Dont expose it if its not serving a purpose. :)

I'm using it, I have a benchmark that just calls layout.  I can remove that.

> 
> > Source/WebKit/chromium/src/WebViewImplRenderingBenchmarkController.cpp:61
> > +            layers.prepend(layer->children());
> 
> why are you prepending? you realize that this is a vector so each prepend is super inefficient?

Obviously, yes, I just couldn't find a doubly-linked list in WTF and I need to prepend for breadth-first-search (which makes applying layer transformations easier).  I just talked to Julian and he pointed me towards Deque, which looks like it doesn't copy on prepend

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