There is an unclear relation between WebContext and {VibrationProvider, BatteryProvider, NetworkInfoProvider} VibrationProvider has WKRetainPtr<WKVibrationRef> m_wkVibrationRef which has reference for object WebVibrationProxy. But the WebVibrationProxy instance is a member of WebContext which also has RefPtr<WebVibrationProxy> Two classes are sharing the reference for WebVibrationProxy but there is no relation between the two. This can make a problem that, When WebContext if deleted befor the VibrationProvider, WebVibrationProxy will be alive without WebContext, because VibrationProvider also has the reference for WebVibrationProxy. This is a problem because WebVibrationProxy should be a member of WebContext. To prevent this, there should be a relation between VibrationProvider and WebContext. (And same about BatteryProvider, NetworkInfoProvider)
Created attachment 166142 [details] Patch
Comment on attachment 166142 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166142&action=review > Source/WebKit2/ChangeLog:3 > + [EFL][WK2] Make relation between WebContext and {VibrationProvider,BatteryProvider,NetworkInfoProvider}. The bug title is really not clear. > Source/WebKit2/ChangeLog:16 > + This can make a problem that, when WebContext is deleted befor the "before" > Source/WebKit2/ChangeLog:25 > + Same about BatteryProvider and NetworkInfoProvider. "Same for" > Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:89 > + vibrationProvider = VibrationProvider::create(context.get()); It looks like we are missing the networkInfoProvider, right? We should probably fix this in a separate patch.
(In reply to comment #2) > (From update of attachment 166142 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=166142&action=review > > > Source/WebKit2/ChangeLog:3 > > + [EFL][WK2] Make relation between WebContext and {VibrationProvider,BatteryProvider,NetworkInfoProvider}. > > The bug title is really not clear. > > > Source/WebKit2/ChangeLog:16 > > + This can make a problem that, when WebContext is deleted befor the > > "before" > > > Source/WebKit2/ChangeLog:25 > > + Same about BatteryProvider and NetworkInfoProvider. > > "Same for" > > > Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:89 > > + vibrationProvider = VibrationProvider::create(context.get()); > > It looks like we are missing the networkInfoProvider, right? We should probably fix this in a separate patch. Addressing the missing networkInfoProvider initialization issue in Bug 97865. This patch will need rebasing afterwards.
Comment on attachment 166142 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166142&action=review >> Source/WebKit2/ChangeLog:3 >> + [EFL][WK2] Make relation between WebContext and {VibrationProvider,BatteryProvider,NetworkInfoProvider}. > > The bug title is really not clear. Yes. How about this? [EFL][WK2] {Vibration,Battery,NetworkInfo}Provider should contain WKContextRef instead of proxy. >> Source/WebKit2/ChangeLog:16 >> + This can make a problem that, when WebContext is deleted befor the > > "before" Oops~ >> Source/WebKit2/ChangeLog:25 >> + Same about BatteryProvider and NetworkInfoProvider. > > "Same for" Ok >> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:89 >> + vibrationProvider = VibrationProvider::create(context.get()); > > It looks like we are missing the networkInfoProvider, right? We should probably fix this in a separate patch. Yes I couldn't find a code about networkInfoProvider. it should be fixed in other patch :)
(In reply to comment #4) > (From update of attachment 166142 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=166142&action=review > > >> Source/WebKit2/ChangeLog:3 > >> + [EFL][WK2] Make relation between WebContext and {VibrationProvider,BatteryProvider,NetworkInfoProvider}. > > > > The bug title is really not clear. > > Yes. How about this? > [EFL][WK2] {Vibration,Battery,NetworkInfo}Provider should contain WKContextRef instead of proxy. Yes, I think it is better. > >> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:89 > >> + vibrationProvider = VibrationProvider::create(context.get()); > > > > It looks like we are missing the networkInfoProvider, right? We should probably fix this in a separate patch. > > Yes I couldn't find a code about networkInfoProvider. it should be fixed in other patch :) This has been fixed in 97865, please rebase.
(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 166142 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=166142&action=review > > > > >> Source/WebKit2/ChangeLog:3 > > >> + [EFL][WK2] Make relation between WebContext and {VibrationProvider,BatteryProvider,NetworkInfoProvider}. > > > > > > The bug title is really not clear. > > > > Yes. How about this? > > [EFL][WK2] {Vibration,Battery,NetworkInfo}Provider should contain WKContextRef instead of proxy. > > Yes, I think it is better. > > > >> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:89 > > >> + vibrationProvider = VibrationProvider::create(context.get()); > > > > > > It looks like we are missing the networkInfoProvider, right? We should probably fix this in a separate patch. > > > > Yes I couldn't find a code about networkInfoProvider. it should be fixed in other patch :) > > This has been fixed in 97865, please rebase. Ok~ I'll apply it :)
Created attachment 166186 [details] Patch
Comment on attachment 166186 [details] Patch Need apply bug 97528
Comment on attachment 166186 [details] Patch Attachment 166186 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/14054286
Created attachment 166209 [details] Patch
Comment on attachment 166209 [details] Patch LGTM, thanks.
Comment on attachment 166209 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166209&action=review > Source/WebKit2/ChangeLog:16 > + This can make a problem that, when WebContext is deleted before the This can become a problem when We... > Source/WebKit2/ChangeLog:23 > + To prevent this, changed the VibrationProvider to have To prevent this, I changed
(In reply to comment #12) > (From update of attachment 166209 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=166209&action=review > > > Source/WebKit2/ChangeLog:16 > > + This can make a problem that, when WebContext is deleted before the > > This can become a problem when We... > > > Source/WebKit2/ChangeLog:23 > > + To prevent this, changed the VibrationProvider to have > > To prevent this, I changed Ok~ I'll change it.
Created attachment 166260 [details] Patch
Created attachment 166262 [details] Patch
Comment on attachment 166262 [details] Patch Clearing flags on attachment: 166262 Committed r129922: <http://trac.webkit.org/changeset/129922>
All reviewed patches have been landed. Closing bug.