102657 - Turbo mode should use an API to check availablity, not an #ifdef

Status: VERIFIED FIXED

(Core Graveyard :: QuickLaunch (AKA turbo mode), defect)

Description

[Mike Kaply [:mkaply]]

Currently, in nsAppRunner.cpp, there is an #ifdef to make turbo mode 
work. This is wrong. This means that any time a platform adds turbo 
mode it has to modify this #ifdef.

This bug adds a new API, supportsServerMode, to nsNativeAppSupport.

This allows any platform that wants to support turbo mode to simply 
override this method and set the value to true.

Diff attached for Windows and Os/2

Comment 1

[Mike Kaply [:mkaply]]

Attachment: Add a new API to check for server mode availability

Comment 2

[Mike Kaply [:mkaply]]

Copying dp since he did this work in nsAppRunner

Comment 3

[sairuh (rarely reading bugmail)]

->turbo/quicklaunch

Comment 4

[Mike Kaply [:mkaply]]

Comment on attachment 51647 [details] [diff] [review]
Add a new API to check for server mode availability

My fix is overkill for this problem.

Since GetIsServerMode returns FALSE in the default case and GetShouldShowUI returns TRUE in the default case, we don't need to check anything else at all.

This #ifdef should simply be removed. All platforms will work fine without this #ifdef and then if someone wants to implement turbo mode, they just modify their nsNativeAppSupportPlatform

This is critical to get turbo mode working on OS/2.

Comment 5

[Bill Law]

Mike, I think it is not mandatory that a platform have an implementation of
nsINativeAppSupport.  Because of that, nsIAppShellService::GetNativeAppSupport
might return NULL.  I think you should modify the newly-exposed code to check
for that before calling through the nativeApp pointer.

Comment 6

[Mike Kaply [:mkaply]]

if GetNativeAppSupport returns NULL, wouldn't the NS_SUCCEEDED on rv fail?

Or is there a case where you would get NS_SUCCEEDED yet nativeApp would be NULL?

Comment 7

[Mike Kaply [:mkaply]]

More info.

Platforms must have an nsNativeAppSupport to implement NS_CreateSplashScreen, 
NS_CanRun, and NS_CreateNativeAppSupport.

Secondly, there is other code in nsAppRunner that relies on nativeAppSupport as 
well:

http://lxr.mozilla.org/seamonkey/source/xpfe/bootstrap/nsAppRunner.cpp#898
http://lxr.mozilla.org/seamonkey/source/xpfe/bootstrap/nsAppRunner.cpp#1278

And this code is not in an #ifdef

The solution here is to remove the #ifdef XP_WIN32.

Comment 8

[Bill Law]

You're right.  Conrad changed nsAppShellService::GetNativeAppSupport a while
back so it doesn't return NS_OK for a null/missing nsINativeAppSupport.

So r=law on the removal of the #ifdef (for which you should really attach a new
patch, even though it is a trivial change).

Comment 9

[Mike Kaply [:mkaply]]

Attachment: Final patch - just remove the #ifdef

Comment 10

[Bill Law]

Comment on attachment 52244 [details] [diff] [review]
Final patch - just remove the #ifdef

r=law

Comment 11

[Blake Ross]

sr=blake

Comment 12

[Bill Law]

fixed

Comment 13

[Grace Bush]

verified code fix