377166 - Mac applicationDockMenu delegate method not called after process restart

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Colin Barrett [:cbarrett]]

I'm having trouble determining exactly *what* the problem is, but I do know that when Firefox has to do a relaunch (e.g. the first time you run a fresh build), the App delegate is not getting set properly.

http://mxr.mozilla.org/seamonkey/source/toolkit/xre/nsAppRunner.cpp#2866

That is where we're is currently calling out and setting the mac application delegate (it's used for installing ourselves as handlers to various Apple Events for opening files).

I'm not entirely sure how this will manifest itself in a build a user would run (first run?).

Comment 1

[Steven Michaud [:smichaud] (Retired)]

Actually, in my tests the NSApp delegate does get set (correctly, on
the second "bounce", to MacApplicationDelegate) when Firefox
relaunches.  And the applicationShouldTerminate: delegate method does
get called (in my patch to bug 377506).  But the applicationDockMenu:
delegate method (which should get called every time Dock application
menu is opened) _doesn't_ get called.

Bug 364326 and bug 368336 only happen when Firefox relaunches, and
appear to be caused by the above-mentioned problem with the
applicationDockMenu: message.  So this bug (377166) may block bugs
364326 and 368336.

I haven't yet figured out why the applicationDockMenu: message doesn't
get called when Firefox relaunches.  But I have a couple more tricks
up my sleeve, and will be spending the next couple of days trying to
find out.

Comment 2

[Steven Michaud [:smichaud] (Retired)]

Attachment: Preliminary patch (hackish)

Here's a preliminary patch that fixes this problem in my tests.  But
it's quite possible that I'll have to change it (and even change the
module where I put it) before I'm done with it.  I hope the reasons
for this will become clear :-)

The ultimate cause of this problem is an Apple design flaw:  Apple has
two sets of published APIs for programming Dock menus (one for Cocoa
apps that uses the NSApplication delegate applicationDockMenu: method,
and a more powerful one for Carbon apps that uses
GetApplicationDockTileMenu() and SetApplicationDockTileMenu() (from
the "Dock Manager Reference"), GetWindowDockTileMenu() and
SetWindowDockTileMenu() (from the "Window Manager Reference") and the
Carbon events kEventAppGetDockTileMenu and kEventWindowGetDockTileMenu
(from the "Carbon Event Manager Reference")).  But these two Dock APIs
don't interoperate well (or possibly at all), you must "choose" one or
the other, and the "choice" tends to be made in quite an arbitrary
manner.

Minefield as it now stands (using Cocoa widgets) normally "chooses" to
use the Cocoa Dock API.  But when Minefield restarts (and its Dock
icon does the infamous double-bump or double-bounce), Minefield
instead "chooses" the Carbon Dock API.

As best I can tell, the "choice" is determined by "who" makes the
first call to the following undocumented method (it's part of the
totally undocumented CoreDock API, which is defined in the HIServices
framework):

long CoreDockRegisterClientWithRunLoop(CoreDockMessageProc inProc,
                                       void *inUserData,
                                       CFRunLoopRef inRunLoop);

This method "registers" a callback (inProc) to receive CoreDock
"messages" (which appear to mostly be related to Dock menus).  The
higher-level Cocoa and Carbon Dock APIs each have their own callback:

CoreDockMessageProc _NSCoreDockMessageReceive; //AppKit framework
CoreDockMessageProc DockCallback; //HIToolbox framework

But, while CoreDockRegisterClientWithRunLoop() is always (in
Cocoa-widget Minefield) called at least twice (once on behalf of each
Dock API), only the first call has any effect.  And (as far as I can
tell) there's no way (documented or undocumented) to reset the
"registered client" to NULL (so that the next call to
CoreDockRegisterClientWithRunLoop() _would_ have an effect).

So the "choice" depends entirely on "who" gets in the first call to
CoreDockRegisterClientWithRunLoop().

Here are the two methods from which
CoreDockRegisterClientWithRunLoop() normally gets called:

BOOL _NSDoOneTimeDockRegistration(void); //AppKit, for Cocoa
void _FirstEventTime(void); //HIToolbox, for Carbon

As far as I can tell, _NSDoOneTimeDockRegistration() is only ever
called from a Cocoa context (from [NSApplication _registerWithDock],
which is in turn called on the first call to [NSApplication
sharedApplication]).  But _FirstEventTime() can be called from either
a Carbon or a Cocoa context (e.g. [NSApplication
nextEventMatchingMask]).  Fortunately, usually the first thing you do
in a Cocoa app is to call [NSApplication sharedApplication] (which
does all kinds of needed initialization).

My patch arranges that _NSDoOneTimeDockRegistration() is always called
(from [NSApplication sharedApplication]) before _FirstEventTime().
But I'm not sure there aren't other ways that are more stable and
predictable.  I'll be looking for them.

(I'll also try to find out why _NSDoOneTimeDockRegistration() doesn't
get called first on the double-bump.)

Comment 3

[Steven Michaud [:smichaud] (Retired)]

I've just discovered that my "preliminary patch" (attachment 264184 [details] [diff] [review])
doesn't work in a shared build (a debug build) -- when a Minefield
shared build restarts it still "chooses" the Carbon Dock API.

It does work in a static build (so people can test with it).  But it's
clear that I will need to revise it.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Attachment: Revised patch (hopefully final)

Here's a better patch (hopefully the final one).  It works in both a
static build and a shared build.

Looking through XRE_main() in toolkit/src/nsAppRunner.cpp, I
discovered that, on relaunch, a call is made to ReceiveNextEvent().
This is one of the ways to invoke a call to _FirstEventTime().  And
since it happens before the first call to [NSApplication
sharedApplication], it causes the browser (on the second bump) to
choose the "Carbon Dock API".

I've fixed the problem by adding a call to [NSApplication
sharedApplication] before the call to ReceiveNextEvent().

Comment 5

[Benjamin Smedberg]

Comment on attachment 264246 [details] [diff] [review]
Revised patch (hopefully final)

Why do we need the MOZ_WIDGET_COCOA ifdef? IIUC, we only use cocoa widgets on trunk for macosx, so we don't need the ifdef.

Comment 6

[Steven Michaud [:smichaud] (Retired)]

> Why do we need the MOZ_WIDGET_COCOA ifdef?

To be consistent with the rest of nsAppRunner.cpp -- it's used in a
couple of other places in this file.

Comment 7

[Steven Michaud [:smichaud] (Retired)]

>> Why do we need the MOZ_WIDGET_COCOA ifdef?
>
> To be consistent with the rest of nsAppRunner.cpp -- it's used in a
> couple of other places in this file.

Though it's true that my patch is the only place a MOZ_WIDGET_COCOA
ifdef is used inside an XP_MACOSX ifdef.

So then ... should I keep the MOZ_WIDGET_COCOA ifdef or get rid of it?

Comment 8

[Josh Aas]

landed on trunk without the MOZ_WIDGET_COCOA stuff

Comment 9

[Ray Kiddy]

I am trying to launch instances of Firefox that I have built from the command-line. Even after checking out this morning, I am still seeing this problem.

The symptoms are that I launch in Terminal, and

1) sometimes I do not get a window or a Dock icon

2) other times I do get a window, but I do not get a Dock icon and the browser will not accept key press events and the menu reflects Terminal, not Minefield.

I can start FF 2.0.0.3 on the command line and it works fine. Are you all sure you are done with this bug?

Comment 10

[Steven Michaud [:smichaud] (Retired)]

I often start Minefield from the command line, and I've never had
anything like the trouble you describe.  But I always make the
distro's Contents/MacOS directory the current directory, and run
"./firefox".

Do you do things differently?

Comment 11

[Colin Barrett [:cbarrett]]

I do this all the time as well, 

./objdir/dist/MinefieldDebug.app/Contents/MacOS/firefox

Is there another bug for what you're seeing, Ray, with full steps to reduce?

Comment 12

[Ray Kiddy]

I cannot believe how stupid this was. Or I was, perhaps.

My script was launching Firefox with:

    /Users/ray/mo/trowser/mozilla/dist/Minefield.app/Contents/Macos/firefox

This launches Firefox, because HFS+ is case-insensitive file system, but it does not set up the menus and messes up the app.

Launching with this works:

    /Users/ray/mo/trowser/mozilla/dist/Minefield.app/Contents/MacOS/firefox