[Webkit-unassigned] [Bug 138093] [GTK] Move RedirectedXCompositeWindow from platform to WebKit2 layer

[bugzilla-daemon at webkit.org]

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

--- Comment #9 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 240542
  --> https://bugs.webkit.org/attachment.cgi?id=240542
Cleanup diffs to make it easier to review

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

>> Source/WebKit2/UIProcess/gtk/RedirectedXCompositeWindow.cpp:143
>> +    ASSERT(supportsXDamageAndXComposite(display));
> 
> I think it's a bug to put this into an ASSERT. This means that this check is only run for debug builds and will crash when run on an XServer that does not support these extensions. Instead we should fail gracefully in this case.

No, it's not. The thing is that previously we had the constructor private, and you could only create the object using the create factory method. Now we are removing the factory methods in WebKit in favor of using std::make_unqiue, but in this particular case, the factory method is required, because it calls supportsXDamageAndXComposite() to return NULL in case xserver doesn't support the required extensions. I still moved to use std::unique_ptr and std::make_unique, but keeping the factory method to check the xserver extensions. But std::make_unique requires the constructor to be public, so this assert is a way to make sure that the caller has used the factory method to create the object. I can remove the ASSERT here, and make the constructor private again, by not using make_unique and returning a std::unique_ptr<RedirectedXCompositeWindow>(new RedirectedXCompositeWindow()); directly.

>> Source/WebKit2/UIProcess/gtk/RedirectedXCompositeWindow.cpp:267
>>  }
> 
> It's a bit odd to move the drawing operation here, and I greatly preferred it exterior to the class. Let the user of the interface decide how to use the surface.

Ok. I made this to simplify the caller code, but it's also true that it may only fit in the current caller code.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141030/d0870928/attachment-0002.html>