EFL and GTK ports have owned "navigatorContentUtilsClient" member variable. However, it is only used for passing a pointer of NavigatorContentUtilsClient. It is useless. To remove it, we need to change parameter type to OwnPtr|PassOwnPtr in NavigatorContentUtils classes. Besides OwnPtr|PassOwnPtr help to manage the client pointer more safely.
Created attachment 228153 [details] Patch for ews
Comment on attachment 228153 [details] Patch for ews Child class of RefCountedSupplement can't use std::unique_ptr yet, because RefCountedSupplement still use PassRefPtr template. It would be good if we change it on new bug. Let me file a bug for it after landing this patch.
Comment on attachment 228153 [details] Patch for ews The patch itself looks good but it should use std::unique_ptr instead of OwnPtr/PassOwnPtr!
(In reply to comment #3) > (From update of attachment 228153 [details]) > The patch itself looks good but it should use std::unique_ptr instead of OwnPtr/PassOwnPtr! As I said, comment #2, we need to modify RefCountedSupplement to adjust std::unique_ptr. However, some classes are inherited it. So, I would like to change them with OwnPtr/PassPwnPtr first, then I would adjust the std::unique_ptr all together. Do you want change those at once ?
(In reply to comment #2) > Child class of RefCountedSupplement can't use std::unique_ptr yet, because RefCountedSupplement still use PassRefPtr template. I don’t understand the connection between PassRefPtr and OwnPtr. Why can classes derived from RefCountedSupplement use OwnPtr safely? Shouldn’t it be RefPtr?
Created attachment 228696 [details] Patch
(In reply to comment #3) > (From update of attachment 228153 [details]) > The patch itself looks good but it should use std::unique_ptr instead of OwnPtr/PassOwnPtr! Ok, I apply std::unique_ptr to EFL port's implementation first before changing OwnPtr|PassOwnPtr in whole NavigatorContentUtils. It looks I need to have time to replace all.
(In reply to comment #5) > (In reply to comment #2) > > Child class of RefCountedSupplement can't use std::unique_ptr yet, because RefCountedSupplement still use PassRefPtr template. > > I don’t understand the connection between PassRefPtr and OwnPtr. Why can classes derived from RefCountedSupplement use OwnPtr safely? Shouldn’t it be RefPtr? It looks there was misunderstanding. I knew we can't pass OwnPtr to RefPtr at that time. So, I thought that RefCountedSupplement needs to be modified as well. But, there seems to be no need to modify it. Sorry for noise.
Comment on attachment 228696 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=228696&action=review > Source/WebKit/efl/ewk/ewk_view.cpp:-263 > - OwnPtr<WebCore::NavigatorContentUtilsClientEfl> navigatorContentUtilsClient; This data member is still needed. It can be a std::unique_ptr, but we can’t just get rid of it. > Source/WebKit/efl/ewk/ewk_view.cpp:687 > + WebCore::provideNavigatorContentUtilsTo(priv->page.get(), std::make_unique<WebCore::NavigatorContentUtilsClientEfl>(smartData->self).get()); This is not going to work. It’s going to pass a pointer to provideNavigatorContentUtilsTo after deleting the NavigatorContentUtilsClientEfl object.
Created attachment 228714 [details] Patch
(In reply to comment #9) > (From update of attachment 228696 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=228696&action=review > > > Source/WebKit/efl/ewk/ewk_view.cpp:-263 > > - OwnPtr<WebCore::NavigatorContentUtilsClientEfl> navigatorContentUtilsClient; > > This data member is still needed. It can be a std::unique_ptr, but we can’t just get rid of it. > > > Source/WebKit/efl/ewk/ewk_view.cpp:687 > > + WebCore::provideNavigatorContentUtilsTo(priv->page.get(), std::make_unique<WebCore::NavigatorContentUtilsClientEfl>(smartData->self).get()); > > This is not going to work. It’s going to pass a pointer to provideNavigatorContentUtilsTo after deleting the NavigatorContentUtilsClientEfl object. Oops, my mistake. thank you for pointing it out. Fixed.
Created attachment 228715 [details] Patch
Comment on attachment 228715 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=228715&action=review > Source/WebKit/efl/ewk/ewk_view.cpp:867 > + priv->navigatorContentUtilsClient = nullptr; This line of code is not needed. Calling "delete priv" below will take care of it.
Created attachment 228718 [details] Patch for landing
Comment on attachment 228718 [details] Patch for landing Clearing flags on attachment: 228718 Committed r166866: <http://trac.webkit.org/changeset/166866>
All reviewed patches have been landed. Closing bug.