[chromium] Add chromium port API for accessing Web Timing information
Created attachment 66976 [details] Patch
Comment on attachment 66976 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=66976&action=review comments are mainly minor > WebKit/chromium/src/WebFrameImpl.h:94 > + virtual WebPerformance performance() const; any reason why this is not virtual WebPerformance& > WebKit/chromium/src/WebPerformance.cpp:44 > + shouldn't this be WebPerformance::WebPerformance(const WebPerformance& p) : m_private(p) > WebKit/chromium/src/WebPerformance.cpp:55 > + i think this happens automatically in C++ unless you say explicit in the constructor > WebKit/chromium/src/WebPerformance.cpp:154 > + this should be at the top with the other constructor > WebKit/chromium/src/WebPerformance.cpp:160 > + ditto with the other operator=
Comment on attachment 66976 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=66976&action=review > WebKit/chromium/public/WebPerformance.h:46 > + virtual ~WebPerformance(); this destructor does not need to be virtual. this class is not designed to be subclassed, right? no need to bloat this object with a vtable pointer. > WebKit/chromium/public/WebPerformance.h:47 > + WebPerformance& operator=(const WebPerformance& p); any non-inline methods outside of WEBKIT_IMPLEMENTATION, need to be marked WEBKIT_API. in general, we do not export constructors or destructors. in this case, you should define a void assign(const WebPerformance&) function and call that to implement both the copy-ctor as well as the assignment operator. >> WebKit/chromium/src/WebFrameImpl.h:94 >> + virtual WebPerformance performance() const; > > any reason why this is not > virtual WebPerformance& yeah, it should not be a reference. WebPreference is intended to be allocated on the stack as a temporary value. it works like a RefPtr<T>. see the definition of WebPrivatePtr<T> if you are curious. >> WebKit/chromium/src/WebPerformance.cpp:154 >> + > > this should be at the top with the other constructor no, it is better to list the functions in the .cpp file in the same order in which they appear in the header file.
Created attachment 68451 [details] Patch
Chris, thanks for getting this patch jump-started again :-) Darin, I think I have all the comment addressed, could you take another pass?
Comment on attachment 68451 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68451&action=review > WebKit/chromium/src/WebFrameImpl.cpp:700 > + return WebPerformance(0); nit: no (0) here. just return WebPerformance()
Committed r68141: <http://trac.webkit.org/changeset/68141>