Merge branch 'dgud/wx/fix-cb-cleanup/OTP-11699'

* dgud/wx/fix-cb-cleanup/OTP-11699:
  wx: Test fixes compilation warning
  wx: Do not warn for our own deprecated functions
  wx: Rewrite eventhandling again

1 files changed, 75 insertions, 117 deletions

diff --git a/lib/wx/src/wxe_server.erl b/lib/wx/src/wxe_server.erl
index 04e0d0faf4..aed9dca7ce 100644
--- a/lib/wx/src/wxe_server.erl
+++ b/lib/wx/src/wxe_server.erl
@@ -36,8 +36,8 @@
 	 terminate/2, code_change/3]).
 
 -record(state, {port,cb_port,users,cleaners=[],cb,cb_cnt}).
--record(user,  {events=[], evt_handler}).
--record(event, {object, callback, cb_handler}).
+-record(user,  {events=[]}).
+%%-record(event, {object, callback, cb_handler}).
 
 -define(APPLICATION, wxe).
 -define(log(S,A), log(?MODULE,?LINE,S,A)).
@@ -119,7 +119,7 @@ handle_call(stop,{_From,_},State = #state{users=Users0, cleaners=Cs0}) ->
     Env = get(?WXE_IDENTIFIER),
     Users = gb_trees:to_list(Users0),
     Cs = lists:map(fun({_Pid,User}) ->
-			   spawn_link(fun() -> cleanup(Env,[User], false) end)
+			   spawn_link(fun() -> cleanup(Env,[User]) end)
 		   end, Users),
     {noreply, State#state{users=gb_trees:empty(), cleaners=Cs ++ Cs0}};
 
@@ -178,13 +178,13 @@ handle_info({'DOWN',_,process,Pid,_}, State=#state{users=Users0,cleaners=Cs}) ->
 	Users = gb_trees:delete(Pid,Users0),
 	Env = wx:get_env(),
 	case User of
-	    #user{events=[], evt_handler=undefined} -> %% No need to spawn
+	    #user{events=[]} -> %% No need to spawn
 		case Cs =:= [] andalso gb_trees:is_empty(Users) of
-		    true  -> {stop, normal, State#state{cleaners=Cs}};
-		    false -> {noreply, State#state{users=Users,cleaners=Cs}}
+		    true  -> {stop, normal, State#state{users=Users}};
+		    false -> {noreply, State#state{users=Users}}
 		end;
 	    _ ->
-		Cleaner = spawn_link(fun() -> cleanup(Env,[User],true) end),
+		Cleaner = spawn_link(fun() -> cleanup(Env,[User]) end),
 		{noreply, State#state{users=Users,cleaners=[Cleaner|Cs]}}
 	end
     catch  _E:_R ->
@@ -222,36 +222,26 @@ code_change(_OldVsn, State, _Extra) ->
 log(Mod,Line,Str,Args) ->
     error_logger:format("~p:~p: " ++ Str, [Mod,Line|Args]).
 
-handle_connect(Object, EvData, From, State0 = #state{users=Users}) ->
-    User0 = #user{events=Evs0,evt_handler=Handler0} = gb_trees:get(From, Users),
-    Callback = wxEvtHandler:get_callback(EvData),
-    case Handler0 of
-	#wx_ref{} when Callback =:= 0 ->
-	    CBHandler = Handler0,
-	    Handler = Handler0;
-	undefined when Callback =:= 0 ->
-	    Handler = new_evt_listener(State0, From),
-	    CBHandler = Handler;
-	_ ->
-	    CBHandler = new_evt_listener(State0, From),
-	    Handler = Handler0
-    end,
-    Evs = [#event{object=Object,callback=Callback, cb_handler=CBHandler}|Evs0],
-    User = User0#user{events=Evs, evt_handler=Handler},
-    State1 = State0#state{users=gb_trees:update(From, User, Users)},
-    if is_function(Callback) orelse is_pid(Callback) ->
-	    {FunId, State} = attach_fun(Callback,State1),
-	    Res = wxEvtHandler:connect_impl(CBHandler,Object,
-					    wxEvtHandler:replace_fun_with_id(EvData,FunId)),
-	    case Res of
-		ok     -> {reply,Res,State};
-		_Error -> {reply,Res,State0}
-	    end;
-
-       true ->
-	    Res = {call_impl, connect_cb, CBHandler},
-	    {reply, Res, State1}
-    end.
+handle_connect(Object, #evh{handler=undefined, cb=Callback} = EvData0, 
+	       From, State0) ->
+    %% Callback let this process listen to the events
+    {FunId, State} = attach_fun(Callback,State0),
+    EvData1 = EvData0#evh{cb=FunId},
+    case wxEvtHandler:connect_impl(Object,EvData1) of
+	{ok, Handler} ->
+	    EvData = EvData1#evh{handler=Handler,userdata=undefined},
+	    handle_connect(Object, EvData, From, State);
+	Error ->
+	    {reply, Error, State0}
+    end;
+handle_connect(Object, EvData=#evh{handler=Handler}, 
+	       From, State0 = #state{users=Users}) ->
+    %% Correct process is already listening just register it
+    put(Handler, From),
+    User0 = #user{events=Listeners0} = gb_trees:get(From, Users),
+    User  = User0#user{events=[{Object,EvData}|Listeners0]},
+    State = State0#state{users=gb_trees:update(From, User, Users)},
+    {reply, ok, State}.
 
 invoke_cb({{Ev=#wx{}, Ref=#wx_ref{}}, FunId,_}, _S) ->
     %% Event callbacks
@@ -329,20 +319,6 @@ get_wx_object_state(Pid) ->
 	_ -> ignore
     end.
 
-new_evt_listener(State, Owner) ->
-    #wx_env{port=Port} = wx:get_env(),
-    _ = erlang:port_control(Port,98,<<>>),
-    Listener = get_result(State),
-    put(Listener, Owner),
-    Listener.
-
-get_result(_State) ->
-    receive
-	{'_wxe_result_', Res} -> Res;
-	{'_wxe_error_', Op, Error} ->
-	    erlang:error({Error, {wxEvtHandler, {internal_installer, Op}}})
-    end.
-
 attach_fun(Fun, S = #state{cb=CB,cb_cnt=Next}) ->
     case gb_trees:lookup(Fun,CB) of
 	{value, ID} ->
@@ -367,90 +343,72 @@ delete_fun(FunId, State = #state{cb=CB}) ->
 	    State
     end.
 
-cleanup_evt_listener(U=#user{events=Evs0,evt_handler=Handler}, EvtListener, Object) ->
-    {Evs, Del} = lists:foldl(fun(#event{object=Obj,cb_handler=CBH}, {Acc, Delete})
-				   when CBH =:= EvtListener, Obj =:= Object ->
-				     {Acc, Delete};
-				(Event = #event{cb_handler=CBH}, {Acc, _Delete})
-				   when CBH =:= EvtListener ->
-				     {[Event|Acc], false};
-				(Event, {Acc, Delete}) ->
-				     {[Event|Acc], Delete}
-			     end, {[], true}, Evs0),
-    Del andalso wxEvtHandler:destroy_evt_listener(EvtListener),
-    case Del andalso Handler =:= EvtListener of
-	true ->
-	    U#user{events=Evs, evt_handler=undefined};
-	false ->
-	    U#user{events=Evs}
-    end.
+cleanup_evt_listener(U=#user{events=Evs0}, EvtListener, Object) ->
+    Filter = fun({Obj,#evh{handler=Evl}}) -> 
+		     not (Object =:= Obj andalso Evl =:= EvtListener) 
+	     end,
+    U#user{events=lists:filter(Filter, Evs0)}.
 
-handle_disconnect(Object, Evh, From, State0 = #state{users=Users0}) ->
+handle_disconnect(Object, Evh = #evh{cb=Fun}, From, 
+		  State0 = #state{users=Users0, cb=Callbacks}) ->
     #user{events=Evs0} = gb_trees:get(From, Users0),
-    Fun = wxEvtHandler:get_callback(Evh),
-    case find_handler(Evs0, Object, Fun) of
-	[] -> {reply, false, State0};
+    FunId = gb_trees:lookup(Fun, Callbacks),
+    case find_handler(Evs0, Object, Evh#evh{cb=FunId}) of
+	[] -> 
+	    {reply, false, State0};
 	Handlers ->
-	    case disconnect(Object,Evh, Handlers) of
-		#event{} ->
-		    {reply, true, State0};
-		Result ->
-		    {reply, Result, State0}
+	    case disconnect(Object,Handlers) of
+		#evh{} -> {reply, true, State0};
+		Result -> {reply, Result, State0}
 	    end
     end.
 
-disconnect(Object,Evh,[Ev=#event{cb_handler=Handler}|Evs]) ->
-    try wxEvtHandler:disconnect_impl(Handler,Object,Evh) of
+disconnect(Object,[Ev|Evs]) ->
+    try wxEvtHandler:disconnect_impl(Object,Ev) of
 	true ->  Ev;
-	false -> disconnect(Object, Evh, Evs);
+	false -> disconnect(Object, Evs);
 	Error -> Error
     catch _:_ ->
 	    false
     end;
-disconnect(_, _, []) -> false.
-
-find_handler(Evs, Object, Fun) ->
-    find_handler(Evs, Object, Fun, []).
-
-find_handler([Ev =#event{object=Object,callback=FunReg}|Evs],Object,Search,Acc) ->
-    case FunReg =:= Search of
-	true -> find_handler(Evs,Object,Search,[Ev|Acc]);
-	false when is_function(FunReg), Search =:= 0 ->
-	    find_handler(Evs,Object,Search,[Ev|Acc]);
-	_ ->
-	    find_handler(Evs,Object,Search,Acc)
+disconnect(_, []) -> false.
+
+find_handler([{Object,Evh}|Evs], Object, Match) ->
+    case match_handler(Match, Evh) of
+	false -> find_handler(Evs, Object, Match);
+	Res  -> [Res|find_handler(Evs,Object,Match)]
     end;
-find_handler([_|Evs],Object,Fun,Res) ->
-    find_handler(Evs,Object,Fun,Res);
-find_handler([],_Object,_Fun,Res) ->
-    Res.
+find_handler([_|Evs], Object, Match) ->
+    find_handler(Evs, Object, Match);
+find_handler([], _, _) -> [].
+
+match_handler(M=#evh{et=MET, cb=MCB}, 
+	      #evh{et=ET, cb=CB, handler=Handler}) ->
+    %% Let wxWidgets handle the id matching
+    Match = match_et(MET, ET) 
+	andalso match_cb(MCB, CB),
+    Match andalso M#evh{handler=Handler}.
 
+match_et(null, _) -> true;
+match_et(Met, Et) -> Met =:= Et.
+
+match_cb(none, _) -> true;
+match_cb({value,MId}, Id) ->  MId =:= Id.
 
 %% Cleanup
 %% The server handles callbacks from driver so every other wx call must
 %% be called from another process, therefore the cleaning must be spawned.
 %%
-%% Using Disconnect when we terminate can crash, it is timing releated
-%% but it seems that disconnect on windows that are being deleted are bad.
-%% Since we are terminating the data will be cleaned up anyway.
-cleanup(Env, Data, Disconnect) ->
+cleanup(Env, Data) ->
     put(?WXE_IDENTIFIER, Env),
-    lists:foreach(fun(User) -> cleanup(User, Disconnect) end, Data),
+    Disconnect = fun({Object, Ev}) ->
+			 try wxEvtHandler:disconnect_impl(Object,Ev)
+			 catch _:_ -> ok
+			 end
+		 end,
+
+    lists:foreach(fun(#user{events=Evs}) -> 
+			  [Disconnect(Ev) || Ev <- Evs]
+		  end, Data),
     gen_server:cast(Env#wx_env.sv, {cleaned, self()}),
     normal.
-
-cleanup(#user{events=Evs, evt_handler=Handler}, Disconnect) ->
-    lists:foreach(fun(#event{object=O, callback=CB, cb_handler=CbH}) ->
-			  Disconnect andalso (catch wxEvtHandler:disconnect_impl(CbH,O)),
-			  case is_function(CB) of
-			      true ->
-				  wxEvtHandler:destroy_evt_listener(CbH);
-			      false ->
-				  ignore
-			  end
-		  end, Evs),
-    case Handler of
-	undefined -> ignore;
-	_ ->  wxEvtHandler:destroy_evt_listener(Handler)
-    end,
-    ok.