[Webkit-unassigned] [Bug 73235] [Chromium] Support adding/removing page overlay to WebView

[bugzilla-daemon at webkit.org]

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





--- Comment #26 from xiyuan <xiyuan at chromium.org>  2011-12-01 16:20:22 PST ---
(From update of attachment 117299)
View in context: https://bugs.webkit.org/attachment.cgi?id=117299&action=review

Will update updated patch shortly.

>> Source/WebKit/chromium/public/WebPageOverlayClient.h:37
>> +class WebPageOverlayClient {
> 
> Darin pointed out that this interface should actually be called WebPageOverlay, not Client

Done.

>> Source/WebKit/chromium/public/WebPageOverlayClient.h:43
>> +    virtual WebRect dirtyRect() const = 0;
> 
> I don't think this is quite accurate - what this is really returning is the rectangle that this overlay is interested in painting. There doesn't seem to be any mechanism to deal with changing the dirty rect.
> 
> I think this would be clearer if the addPageOverlay() function specified the bounds of the overlay, just as it specified the z-order, and the rule for changing it was that if you wanted to change the bounds of an overlay you needed to remove it and add it back. That would be consistent with the way you can change the z-order.
> 
> Although looking in detail we aren't using this functionality - every overlay covers the full viewport. So let's punt on this functionality for now and add it when we have a need.

Agree with your assessments.

>> Source/WebKit/chromium/public/WebView.h:424
>> +    virtual void removeWebPageOverlay(WebPageOverlayClient*) = 0;
> 
> Darin let me know that we don't use the "web" prefix in the function names. So these should be called:
> addPageOverlay(...)
> removePageOverlay(...)
> 
> see the setVisibilityState() above as an example. We all apologize deeply for the inconsistencies :/

Done.

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