7571 - [Dogfood]MacOS activate/deactivate events are getting dropped

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect, P1)

Description

[tague]

Ocassionally, MacOS window activate and deactivate event messages are getting
dropped.  I rely on the window activate and deactivate to activate/deactivate
the TSMDocument which corresponds to that window.

If the activate and deactivate events aren't delivered correctly, the TSM
documents get out of synch and that is really bad (tm).

Comment 1

[karnaze (gone)]

Pierre, I hopw you know what this is about.

Comment 2

[Pierre Saslawsky]

I don't understand what you mean by "events are occasionally getting dropped".
Tague, could you describe more accurately what you have seen and what you did to
detect it? Any idea as to where they are dropped or why it is occasional?

Comment 3

[tague]

Ocassionally = not 100% reproduceable

What I am seeing is that MacOS activate/deactivate window events are not always
being delivered to the nsMacEventHandler's ActivateEvent handler.  I can't
reproduce it 100% of the time, but one of the symptoms is that a TSMDocument
will get created but never activated -- this causes an assert when the window
changes because the handler deactivates a non-active TSMDocument.  You can also
watch the log, and you will notice that there are TSM documents being created
which arent activated.

I don't know why this only happens occasionally.  I haven't found the cause.
Pinkerton suggested that it might be a timing problem.  I could be related to
the sidebar refreshing itself.

Comment 4

[Pierre Saslawsky]

I'm not sure your interpretation of the symptoms is the correct one. The path is
the following and it seems fairly straightforward to me:
- activateEvt
- nsMacMessagePump::DispatchEvent
- nsMacMessagePump::DoActivate()
- nsMacMessagePump::DispatchOSEventToRaptor
- nsMacMessageSink::DispatchOSEvent
- nsMacWindow::HandleOSEvent
- nsMacEventHandler::HandleOSEvent
- nsMacEventHandler::HandleActivateEvent

If you have a little time to investigate this problem, could you put a counter in
nsMacMessagePump::DispatchEvent() when an activateEvt is received, then another
counter in nsMacEventHandler::HandleActivateEvent and check whether the events
really are dropped somewhere in between?

If you can't do it, could you tell me what to do to be more likely to reproduce
the problem (viewer or apprunner? one or several windows? what url? etc...)

Comment 5

[Pierre Saslawsky]

Oh... and what should I do and how long should I try to reproduce it?

Comment 6

[Pierre Saslawsky]

Ok... I did the test with a counter: it's incremented in
nsMacMessagePump::DispatchEvent() and decremented in
nsMacEventHandler::HandleActivateEvent(). I checked and it's always 0 after
decrementing it.

It looks like your explanation is not correct but without description from your
side about how to reproduce the problem and what to look for exactly when it
happens, I can't investigate more.

CCd myself and reassigned to Tague.

Comment 7

[Pierre Saslawsky]

Tague, please complete the bug report with the requested information before
reassigning back to me. As I wrote, I can't investigate much more if you don't
provide this information. If you don't understand what information I need, please
give me a call (x2340). I tried to reach you twice today but you were not there.

I believe I checked everything on the even handling side. The problem you are
seeing is maybe in your code or inside TSM in the Mac Toolbox but since you are
the person who did the implementation of TSM, you are the most competent to debug
any problem with it.

Comment 8

[Pierre Saslawsky]

I looked at your bug list and there is another bug related to this one that you
did not mention before. It mentions something about switching to the console
window. Maybe the problem you are seeing only comes from the console window...

Could you disable the 2 lines "if (SIOUXHandleOneEvent(anEvent)) return;" in
nsMacMessagePump.cpp and check if you still see the bug?

Comment 9

[tague]

i'm not sure what bug you a referring to.  can you give me the bug number?

Comment 10

[Pierre Saslawsky]

It's bug #7365: "DeactiveTSMDocument Failed Assertion".

Comment 11

[tague]

Pierre :-

Your comment doesn't match the bug you referenced.  You said "It mentions
something about switching to the console window. Maybe the problem you are
seeing only comes from the console window..."

Nothing in bug #7365 refers to the console window.  Bug #7365 is essentially the
symptoms of this bug.

Comment 12

[cyrus]

Au contraire. I quote myself from 7365:

'When switching to the console window, or attempting to start a new browser
window, I sometimes see an assertion nsMacEventHandler::HandleActivateEvent. The
assertion says "DeactivateTSMDocument Failed Assertion"'

Comment 13

[leger]

Moving all Widget Set bugs, past and present, to new HTML Form Controls
component per request from karnaze.  Widget Set component will be retired
shortly.

Comment 14

[tague]

moving to M10 to reflect bug fixing priorities

Comment 15

[leger]

Adding [e-mail] to cc list.

Comment 16

[Frank Tang]

tague, is this related to 15708 ?

Comment 17

[tague]

bulk reassign bugs to ftang for now

Comment 18

[Pierre Saslawsky]

Marking P1/blocker/M11/[dogfood]/[PDT+] like #15708

Comment 19

[Pierre Saslawsky]

*** Bug 15708 has been marked as a duplicate of this bug. ***

Comment 20

[Pierre Saslawsky]

Marking dependency on #16127

Comment 21

[Pierre Saslawsky]

I marked #15708 as duplicate of this bug consecutively to the following email
exchange:

Steve Clark wrote:
   >
   > concerning bug http://bugzilla.mozilla.org/show_bug.cgi?id=15708...
   >
   > pierre's last comment on 10/12 seemed to indicate he had a good idea what
   > the problem might be, but there's been no comments in the bug from him or
   > chris since.  what's the latest?  have you two had a chance to talk it
   > through yet?
   >

   We did not talk. I tried to reach Chris one day and then I must have
   forgotten... Anyhow, I looked a bit more at the bug and it's not what I
   thought it was. It's in fact a duplicate of #7571 and 7365.

   We have two TSM functions (ActivateTSMDocument / DeactivateTSMDocument) that
   must be called when a window is activated or deactivated and I am pretty sure
   that the activate events generated by the OS are indeed dispatched to the
   windows. Tague thought they were not, and this is a hypothesis he put forward
   several months ago but which was already dismissed at that time. I don't
   understand why it's coming back now. As Joe Francis wrote last week, it's
   more likely a problem in TSM - or in the use we do of TSM - but I don't think
   someone has looked into it yet.

   For the full story, you can visit the two other bugs which have been opened
   on that matter. They have been in lethargy for the past few months:
           http://bugzilla.mozilla.org/show_bug.cgi?id=7571
           http://bugzilla.mozilla.org/show_bug.cgi?id=7365
Pierre

Comment 22

[Frank Tang]

I dig into it by apply the foolowing patch and find out the following-



Index: nsMacEventHandler.cpp

===================================================================

RCS file: /cvsroot/mozilla/widget/src/mac/nsMacEventHandler.cpp,v

retrieving revision 1.64

diff -c -2 -r1.64 nsMacEventHandler.cpp

*** nsMacEventHandler.cpp	1999/10/27 03:32:31	1.64

--- nsMacEventHandler.cpp	1999/11/02 00:11:08

***************

*** 31,34 ****

--- 31,35 ----

  #include "nsIRollupListener.h"



+ #define DEBUG_TSM

  extern nsIRollupListener * gRollupListener;

  extern nsIWidget         * gRollupWidget;

***************

*** 202,206 ****

  	NS_ASSERTION(err==noErr,"nsMacEventHandler::nsMacEventHandler:

NewTSMDocument failed.");



! #if 0

  	printf("nsMacEventHandler::nsMacEventHandler: created TSMDocument[%p]\

n",mTSMDocument);

  #endif

--- 203,207 ----

  	NS_ASSERTION(err==noErr,"nsMacEventHandler::nsMacEventHandler:

NewTSMDocument failed.");



! #ifdef DEBUG_TSM

  	printf("nsMacEventHandler::nsMacEventHandler: created TSMDocument[%p]\

n",mTSMDocument);

  #endif

***************

*** 862,868 ****

  		if (mTSMDocument)

  			err = ::ActivateTSMDocument(mTSMDocument);

  #if 0

  		NS_ASSERTION(err==noErr,"nsMacEventHandler::HandleActivateEvent:

ActivateTSMDocument failed");

! 		printf("nsEventHandler::HandleActivateEvent: ActivateTSMDocument[%p]\

n",mTSMDocument);

  #endif



--- 863,872 ----

  		if (mTSMDocument)

  			err = ::ActivateTSMDocument(mTSMDocument);

+ #ifdef DEBUG_TSM

  #if 0

  		NS_ASSERTION(err==noErr,"nsMacEventHandler::HandleActivateEvent:

ActivateTSMDocument failed");

! #endif

! 		printf("nsEventHandler::HandleActivateEvent: ActivateTSMDocument[%p] %s

return %d\n",mTSMDocument,

! 		(err==noErr)? "" : "ERROR!!!", err);

  #endif



***************

*** 901,907 ****

  		if (mTSMDocument)

  			err = ::DeactivateTSMDocument(mTSMDocument);

  #if 0

  		NS_ASSERTION(err==noErr,"nsMacEventHandler::HandleActivateEvent:

DeactivateTSMDocument failed");

! 		printf("nsEventHandler::HandleActivateEvent: DeactivateTSMDocument[%p]\

n",mTSMDocument);

  #endif



--- 905,914 ----

  		if (mTSMDocument)

  			err = ::DeactivateTSMDocument(mTSMDocument);

+ #ifdef DEBUG_TSM

  #if 0

  		NS_ASSERTION(err==noErr,"nsMacEventHandler::HandleActivateEvent:

DeactivateTSMDocument failed");

! #endif

! 		printf("nsEventHandler::HandleActivateEvent: DeactivateTSMDocument[%p]

%s return %d\n",mTSMDocument,

! 		(err==noErr)? "" : "ERROR!!!", err);

  #endif



1. If you have two mozilla window, and you switch from window A to B, you got

Deactive A

Actrive B

which mean there are no problem

2. If you have two mozilla window, and you switch from window A to finder, and

then switch from finder back to window A-

you will got

(Switch from Window A to finder)

[nothing happen, you don't got a Deactive A)

(Switch from finder to Window A)

Deactive A

Active A



The Deactive A will not get called WHEN you switch to finder, but AFTER you

switch back to Window A. And then a seperate Active A will be called. This is a

problem but won't cause TSM confused



3. If you have two mozilla window, and you switch from window A to finder, and

then switch from finder back to window B-

you will got

(Switch from Window A to finder)

[nothing happen, you don't got a Deactive A)

(Switch from finder to Window B)

Deactive A[ --> got a TSM error -2507 - TSM think this window is not active and

you try to deactive it.]

Active B



The Deactive A will not get called WHEN you switch to finder, but AFTER you

switch back to Window B. In that time, the OS think the window A is not active in

that moment, and you try to deactive the TSM which associate with it. So it

complain about it and return a -2507 ( tsmDocNotActiveErr - see IM:Text p 7-107)



Is the timing of the Deactive correct ? Is that cross platfrom ?

Reassign this back to saari. I will check in the above patch when the tree open

and green.

Comment 23

[Pierre Saslawsky]

On the related bug #15708, I wrote:

    I checked again today and I confirm that nsMacEventHandler::
    HandleActivateEvent() and then ActivateTSMDocument() and
    DeactivateTSMDocument() are getting called on activate and
    deactivate events.

    But now the question is: do we also need to call
    ActivateTSMDocument() and DeactivateTSMDocument() on
    suspend/resume events?

It seems to be the case. The best way to fix think, I think, is to call
nsMacEventHandler::HandleActivateEvent() when we receive a suspend/resume event
and modify this function to do something like:

  Boolean isActive;

  switch (aOSEvent.what)
  {
    case activateEvt:
      isActive = ((aOSEvent.modifiers & activeFlag) != 0);
      break;

    case osEvt:
      isActive = ! mInBackground;
      break;
  }
  if (isActive)
  {
    .....
  }

Comment 24

[David Hyatt]

*** Bug 15708 has been marked as a duplicate of this bug. ***

Comment 25

[Peter Trudelle]

What is the status of this bug?  Is it really PDT+?  Is it really a blocker?  If
so, who is unable to do their job until it is fixed?  Do we really need to hold
M11 for this?

Comment 26

[bobj]

The high level: If you switch back and forth between from Seamonkey and
non-Seamonkey (e.g., switch to the Finder or other app), the IME stops
working because TSM state gets screwed up.

Frank and Pierre believe the problem is that ActivateTSMDocument() and
DeactivateTSMDocument() are not being called for suspend/resume events.

Pierre suggested a (simple?) change in his comment on 11/01/99 16:30.
Will ChrisS look at that and comment if he agrees?

Comment 27

[Peter Trudelle]

removing obsolete [blocker] tag and PDT+ (since it was not put there by PDT)

Comment 28

[Peter Trudelle]

After discussing with Saari and Chofmann (couldn't get hold of Pierre), I'm
moving this to M12.  Saari characterizes the fix as risky, and Chofmann has
agreed wait until M12 to take it.

Comment 29

[Peter Trudelle]

Whoops, didn't see that the closed dup was legit PDT+, restoring.
Talked to Pierre, he will stop by and discuss the riskiness of the proposed fix
with Saari.  If saari thinks the fix is okay, could one of the INTL guys apply,
test and check it in?  Saari is Most Doomed right now, so every bit of help is
much appreciated.

Comment 30

[Frank Tang]

pierre make the code change in my local machine. Take this bug from saari back
to ftang.

Comment 31

[leger]

Putting on [PDT+] radar.

Comment 32

[Frank Tang]

fix and check in.

Comment 33

[leger]

Resetting QA Contact.

Comment 34

[Paul MacQuiddy]

marking verified per jfrancis's comments in bug 15708 - I quote "Woohoo!  Thanks
to whoever fixed this." :-)