Timeout callback implementation can be moved to the CallbackDataTimer class.
Created attachment 166150 [details] Patch
Created attachment 166264 [details] Patch
Comment on attachment 166264 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166264&action=review Looks good refactoring. > Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.cpp:85 > + CallbackDataTimer(double timeoutSeconds) Missing explicit keyword
Created attachment 167036 [details] Patch
(In reply to comment #3) > (From update of attachment 166264 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=166264&action=review > > Looks good refactoring. > > > Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.cpp:85 > > + CallbackDataTimer(double timeoutSeconds) > > Missing explicit keyword I added this~
Comment on attachment 167036 [details] Patch LGTM.
Comment on attachment 167036 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167036&action=review > Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.cpp:117 > + virtual void onTimeOut() { } I think it would be good to add onTimeOut() when it is needed actually. Please remove this before landing.
(In reply to comment #7) > (From update of attachment 167036 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=167036&action=review > > > Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.cpp:117 > > + virtual void onTimeOut() { } > > I think it would be good to add onTimeOut() when it is needed actually. Please remove this before landing. Ok~ I'll remove it.
> Ok~ I'll remove it. What is up with all those ~ signs? I have no idea what this might even mean.
(In reply to comment #9) > > Ok~ I'll remove it. > > What is up with all those ~ signs? I have no idea what this might even mean. Do you mean you can't know what this patch mean ? I understood this patch moves timeout callback into the CallbackDataTimer class as member variable. Because waitUntilXXX functions should pass its callback function to ctor of CallbackDataTimer. Beside all callback functions do same work. Do you think this patch is redundant ?
(In reply to comment #10) > (In reply to comment #9) > > > Ok~ I'll remove it. > > > > What is up with all those ~ signs? I have no idea what this might even mean. > > Do you mean you can't know what this patch mean ? I understood this patch moves timeout callback into the CallbackDataTimer class as member variable. Because waitUntilXXX functions should pass its callback function to ctor of CallbackDataTimer. Beside all callback functions do same work. Do you think this patch is redundant ? Maybe kenneth asked about the '~' character that I used in the comment. (is it?) It means nothing, just like dot character. If you have some confusion about the character, I'll not use it :)
Oops, if so, sorry about misunderstanding. :-)
Created attachment 167752 [details] Patch
(In reply to comment #12) > Oops, if so, sorry about misunderstanding. :-) Exactly it is not really used in English so better not confuse people :-) It could be a "." for me or a ";" no idea.
(In reply to comment #14) > (In reply to comment #12) > > Oops, if so, sorry about misunderstanding. :-) > > Exactly it is not really used in English so better not confuse people :-) It could be a "." for me or a ";" no idea. Thanks for your kind comment. I'll remember that :-)
Comment on attachment 167752 [details] Patch Clearing flags on attachment: 167752 Committed r130836: <http://trac.webkit.org/changeset/130836>
All reviewed patches have been landed. Closing bug.