Created attachment 489080 [details] [diff] [review] patch
Comment on attachment 489080 [details] [diff] [review] patch That's one large patch :). smaug should review this one, as this is primarily his code.
ESM is not primary my code. It is mainly way older than my contribution to Mozilla ;) But I'll review.
So this can be done after --disable-libxul isn't supported anymore.
Created attachment 521931 [details] [diff] [review] Refreshed patch This can be done now that we require libxul
Comment on attachment 521931 [details] [diff] [review] Refreshed patch >+ /** >+ * Notify that the given NS_EVENT_STATE_* bit has changed for this content. >+ * @param aContent Content which has changed states >+ * @param aState Corresponding state flags such as NS_EVENT_STATE_FOCUS >+ * @return Whether the content was able to change all states. Returns PR_FALSE >+ * if a resulting DOM event causes the content node passed in >+ * to not change states. Note, the frame for the content may >+ * change as a result of the content state change, because of >+ * frame reconstructions that may occur, but this does not >+ * affect the return value. >+ */ >+ PRBool SetContentState(nsIContent *aContent, nsEventStates aState); >+ void ContentRemoved(nsIDocument* aDocument, nsIContent* aContent); >+ PRBool EventStatusOK(nsGUIEvent* aEvent); > >- NS_IMETHOD SetCursor(PRInt32 aCursor, imgIContainer* aContainer, >- PRBool aHaveHotspot, float aHotspotX, float aHotspotY, >- nsIWidget* aWidget, PRBool aLockCursor); >+ /** >+ * Register accesskey on the given element. When accesskey is activated then >+ * the element will be notified via nsIContent::PerformAccesskey() method. >+ * >+ * @param aContent the given element >+ * @param aKey accesskey >+ */ >+ void RegisterAccessKey(nsIContent* aContent, PRUint32 aKey); >+ >+ /** >+ * Unregister accesskey for the given element. >+ * >+ * @param aContent the given element >+ * @param aKey accesskey >+ */ >+ void UnregisterAccessKey(nsIContent* aContent, PRUint32 aKey); >+ >+ /** >+ * Get accesskey registered on the given element or 0 if there is none. >+ * >+ * @param aContent the given element >+ * @param aKey registered accesskey >+ * @return NS_OK >+ */ >+ virtual PRUint32 GetRegisteredAccessKey(nsIContent* aContent); Why is this virtual? So that external libraries can access it? (Which library does that?) @return is wrong, so is the latter @param. I believe this patch is such that it should be taken to Fx6. So it could land to trunk in a month or so. r+ if the comments are fixed and the virtual is either removed or explained.
Created attachment 527251 [details] [diff] [review] Patch addressing review comments r=smaug Removed the virtual and fixed comments.
Fixed in cedar. Should be synced with mozilla-central in the next 24 hours.
I had to backout from cedar. David, to you have an access to the try server?
Created attachment 527567 [details] [diff] [review] Patch r=smaug Hopefully this lands before it rots again
This may have caused a minor Dromaeo regression.
(In reply to comment #13) > This may have caused a minor Dromaeo regression. Should it be backed out then?
I don't know. Smaug? Dzbarsky? Is anyone looking at the regression?
If it caused a regression we should probably back it out. I don't know how to pinpoint the exact cause.
Yeah, it is quite strange if a patch which make us execute less code, and that less should be even better in the caches, causes regression.
are we sure this caused the regression?
And the code this touches isn't performance-vice too hot.
No, we're not sure. But this was the only one in the range that looked likely. http://groups.google.com/group/mozilla.dev.tree-management/browse_thread/thread/e8cacce1606670f2#
http://hg.mozilla.org/mozilla-central/rev/6cc6b44735a7 looks more likely to me
Dromaeo (jslib) seems to correspond to these pages http://mxr.mozilla.org/mozilla/source/testing/performance/talos/page_load_test/dromaeo/jslib.manifest I had a quick look and there is only one application of border-radius, to b elements, and it does not seem to have overflow different from visible. If that is the case then http://hg.mozilla.org/mozilla-central/rev/6cc6b44735a7 should have no effect. It can still be backed out if people want.