Last Comment Bug 653333 - Remove legacy splash screen code (nsSplashScreen, MOZ_SPLASHSCREEN, splash.bmp) now that no platforms use it
: Remove legacy splash screen code (nsSplashScreen, MOZ_SPLASHSCREEN,
Product: Core
Classification: Components
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla8
Assigned To: Ed Morley [:emorley]
Depends on: 481494 498707 502526 504750 652445
Blocks: 329742 643122 506217
  Show dependency treegraph
Reported: 2011-04-27 19:46 PDT by Ed Morley [:emorley]
Modified: 2011-08-12 15:49 PDT (History)
4 users (show)
emorley: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (21.30 KB, patch)
2011-04-28 07:34 PDT, Ed Morley [:emorley]
dtownsend: review+
benjamin: review+
Details | Diff | Review
Patch v1.1 (22.35 KB, patch)
2011-07-19 13:42 PDT, Ed Morley [:emorley]
dtownsend: review+
Details | Diff | Review
Patch v1.2 (22.38 KB, patch)
2011-08-01 11:50 PDT, Ed Morley [:emorley]
emorley: review+
Details | Diff | Review

Description Ed Morley [:emorley] 2011-04-27 19:46:35 PDT
My patch in bug 652445 (once it lands) removes nsSplashScreenWin.cpp, since it was only used under WinCE.

This leaves all other platforms using nsSplashScreenDummy.cpp:

...which does nothing but return null:

These files can thus be removed:
- /toolkit/xre/nsSplashScreenDummy.cpp
- /toolkit/xre/nsSplashScreen.h

Which leaves MOZ_SPLASHSCREEN unused, so can be removed:

And also several splash.bmp files unused, so can be removed:
Comment 1 Ed Morley [:emorley] 2011-04-27 19:50:05 PDT
Note: The Android splash screen doesn't use nsSplashScreen or MOZ_SPLASHSCREEN; so isn't affected:
Comment 2 Ed Morley [:emorley] 2011-04-28 07:34:34 PDT
Created attachment 528853 [details] [diff] [review]
Patch v1

Remove all #ifdef MOZ_SPLASHSCREEN code & hg rm the following:
- browser/branding/*/splash.bmp
- mobile/app/splash.bmp
- toolkit/xre/nsSplashScreen.h
- toolkit/xre/nsSplashScreenDummy.cpp

Passed try:

Not sure who to ask for review, since this touches a few areas + looking at the hg logs wasn't conclusive. Picked you due to: ; but please let me know if I should be asking someone else. Thanks!

(Note: This patch applies on top of the ones in bug 652445, which haven't yet landed).
Comment 3 Dave Townsend [:mossop] 2011-04-28 11:22:29 PDT
Comment on attachment 528853 [details] [diff] [review]
Patch v1

I'd like Benjamin to assert he's ok with this then I think it's good to go.
Comment 4 Benjamin Smedberg [:bsmedberg] 2011-04-29 07:39:28 PDT
Comment on attachment 528853 [details] [diff] [review]
Patch v1

I am 99% sure that the Android splash screen does not use this code, but please confirm that with mfinkle before landing this.
Comment 5 Ed Morley [:emorley] 2011-05-01 08:26:36 PDT
Mark, bsmedberg has asked that I check with you regarding comment 4. The MXR links in comment 1 make me believe that Android does not use nsSplashScreen - but if you could confirm here, that would be great. Thanks!
Comment 6 Mark Finkle (:mfinkle) (use needinfo?) 2011-05-01 08:45:59 PDT
Android does not use this code for a splashscreen. We use the Java loader code to display a splashscreen, before the Mozilla platform is really even initialized.

However, here is a patch to add splash screen support to maemo/meego and it uses this code. See bug 643122.

Lets hold off on removing the splashscreen support until we see if Meego, and any other new mobile platforms, might need it.
Comment 7 Ed Morley [:emorley] 2011-05-02 17:43:39 PDT
Thanks for the heads up.

Suspect even if bug 643122 lands in it's nsSplashScreen using form, there are going to be unused splash.bmp files from the WinCE era (eg the nightly/aurora splash.bmp are out of date); but will just be easier to wait until the dust settles on that bug before moving this one further...
Comment 8 Ed Morley [:emorley] 2011-07-19 13:42:57 PDT
Created attachment 546887 [details] [diff] [review]
Patch v1.1

Only change from previously r+'d patch are the two lines in mobile/app/ - requesting review just for those. Thanks! :-)
Comment 9 Ed Morley [:emorley] 2011-08-01 11:50:22 PDT
Created attachment 549876 [details] [diff] [review]
Patch v1.2

Updated to tip. No other changes, so carrying forwards r+.
Sent to try again for build only, for sanity check:

See comment 2 for older full try run.

Thanks for the review Dave :-)
Comment 10 Ed Morley [:emorley] 2011-08-05 08:42:25 PDT

Note You need to log in before you can comment on or make changes to this bug.