Bug 114514 – Widget should not depend on AXObjectCache

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Carlos Garcia Campos]

It's a layering violation that Widget depends on AXObjectCache.

[Carlos Garcia Campos]

Created attachment 197855 [details]
Patch

I'm not sure the patch is correct, because previously the scrollbars were removed from the cache on destruction, but I think scrollbars were leaked because the AXObject cache holds a reference, and they were only removed by the cache from the destructor.

[Build Bot]

Comment on attachment 197855 [details]
Patch

Attachment 197855 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/76144

[Carlos Garcia Campos]

(In reply to comment #2)
> (From update of attachment 197855 [details])
> Attachment 197855 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.appspot.com/results/76144

Right, maybe we don't even need didAddScrollbar helper function. I'll fix it.

[Carlos Garcia Campos]

Created attachment 197859 [details]
Updated patch to fix the mac build

[chris fleizach]

Comment on attachment 197859 [details]
Updated patch to fix the mac build

View in context: https://bugs.webkit.org/attachment.cgi?id=197859&action=review

> Source/WebCore/page/FrameView.h:488
> +    virtual void didAddVerticalScrollbar(Scrollbar*) OVERRIDE;

Seems like we could make do with just  two methods, something like...

enum { VERTICAL, HORIZONTAL} ScrollbarType;
virtual void addScrollbar(Scrollbar*, ScrollbarType)
virtual void removeScrollbar(Scrollbar*, ScrollbarType)

[Carlos Garcia Campos]

Created attachment 198133 [details]
Updated patch according to chris suggestion

[Build Bot]

Comment on attachment 198133 [details]
Updated patch according to chris suggestion

Attachment 198133 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/119167

[Build Bot]

Comment on attachment 198133 [details]
Updated patch according to chris suggestion

Attachment 198133 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/161165

[Carlos Garcia Campos]

Created attachment 198138 [details]
Try to fix mac build

[Build Bot]

Comment on attachment 198138 [details]
Try to fix mac build

Attachment 198138 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/161173

[Build Bot]

Comment on attachment 198138 [details]
Try to fix mac build

Attachment 198138 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/119177

[Carlos Garcia Campos]

Comment on attachment 198138 [details]
Try to fix mac build

This is the wrong fix, I didn't noticed that what I changed is ScrollAnimator in mac, which api hasn't changed. The problem is with the exported symbols. so, the previous patch is good, just need to update the symbols. I don't know how the exported symbols files work in mac, so I'll need some help with this.

[Carlos Garcia Campos]

Created attachment 198340 [details]
Another try to fix the mac build

Updated the symbols in WebCore.exp.in

[Build Bot]

Comment on attachment 198340 [details]
Another try to fix the mac build

Attachment 198340 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/171067

[Build Bot]

Comment on attachment 198340 [details]
Another try to fix the mac build

Attachment 198340 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/25138

[Carlos Garcia Campos]

Created attachment 198347 [details]
Try to fix WebKit2 build

Forgot to git grep in WebKit2, fix a couple of cases where the old api was used.

[Carlos Garcia Campos]

All green now, could someone review this, please?

[Carlos Garcia Campos]

Committed r148823: <http://trac.webkit.org/changeset/148823>