Closed Bug 446591 (headless) Opened 16 years ago Closed 2 years ago

Make it possible to run Mozilla with UI "rendered" to memory

Categories

(Core :: Widget, enhancement)

Product:
Core
Component:
Widget
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED DUPLICATE of bug 1338004

People

(Reporter: damiano.albani, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [imvu] [sudweb][tpi:-])

Attachments

(7 files, 6 obsolete files)

The build configuration I'm using with this patch
609 bytes, text/plain
Details
patch for the screenshot sample
748 bytes, patch
Details | Diff | Splinter Review
Flash test page
148.06 KB, image/png
Details
Flash test page debug output from moz-headless-screenshot
2.07 KB, text/plain
Details
Latest diff between mozilla-central and offscreen/headless
667.80 KB, patch
Details | Diff | Splinter Review
imvu exposing RenderDocumentToMemory for 3D overlay rendering
3.34 KB, patch
Details | Diff | Splinter Review
imvu exposing dirty rectangle updates for offscreen windows
3.42 KB, patch
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008061015 Firefox/3.0
Build Identifier: 

It would be nice to be able to run Mozilla apps in a headless environment.
Currently, if one wants to run Firefox on a server to make it render web pages for example, tricks like Xvfb are unfortunately required.

Now that rendering is heavily based on Cairo, there's the opportunity to use its "image surfaces" API (in addition to XLib, Quartz or Win32).

Is there any particular technical difficulty that I haven't foreseen? Maybe plugins direct rendering??

Reproducible: Always
Colleagues of me use the DirectFB interface, although it's based on Firefox 2 for the moment, due to a dependency on MIPS architecture. See bug 422221.
That's *very* interesting, I wasn't aware of that DirectFB porting effort!

However, what's a bit "annoying" for me is that it's limited to the Linux platform (if I'm not mistaken). Mozilla is nice because it doesn't lock on any particular platform -- I'd prefer stay that way and be able to run my XULRunner app on Windows, Linux & Mac :-)

Furthermore, I've just stumbled upon a GTK bug entry which aims at supporting "offscreen rendering" : http://bugzilla.gnome.org/show_bug.cgi?id=318807
I don't know if that would be applicable to the situation.
(In reply to comment #1)
> Colleagues of me use the DirectFB interface, although it's based on Firefox 2
> for the moment, due to a dependency on MIPS architecture. See bug 422221.
> 

Clarification : it's the work of my colleagues (for my daytime job) that is limited to Firefox 2 on MIPS. Firefox 3 with the patches found in bug 422221 (not their job)  works ok. And DirectFB is not really a render-to-memory interface, it's just support for the framebuffer device, which is used in some very simple video hardware (like on mobiles). But it's so simple to read back out, that you can use it to export the memory buffer again. And that's all that I can say about their work.

Off-screen pixmaps are related but different : they're only used for small parts of the screen, and can be seen as a temporary display. It's sometimes used in printer devices or to copy a webpage on the clipboard and such. Not for the full GUI.
The attached patch adds a new backend (that depends on glib/gobject) that allows for headless rendering. It's not complete, but basic functionality works and I've been using it to develop a mozilla-based rendering widget in Clutter (http://clutter-project.org/).

The most limiting thing I've found is that there's no way to register a Listener for document size-changes, or some kind of mode for the scrollframe to have a minimum width/height, but grow to fit its contents.

Feedback would be much appreciated!
Attachment #350162 - Attachment mime type: application/octet-stream → text/plain
Cool!!!

So this was based on the Qt backend, but with GTK and cairo stuff patched in?
This is mostly based on the GTK backend (with GTK bits removed/replaced), though I think I started with the nsWindow.c from Qt as it was a bit easier to start with, and I took influence from the Photon backend in places too. Most of the new code is confined to nsWindow and MozDrawingArea (the latter of which is completely original).

Blog post with a demonstration that uses it: http://chrislord.net/blog/Software/mozilla-clutter.enlighten
Status: UNCONFIRMED → NEW
Ever confirmed: true
So what's the path to getting this into the tree?
It'd be ideal if we could have a headless branch that I (and others) could push changes to and keep in sync with tip. The patch isn't really ready as it is, some stuff is missing, other bits are broken, but it'd make things much easier to not have to maintain a separate external repository (as is done currently).
Will people other than you be hacking on this?  Sounds like it might be a good candidate for an incubator repo.
This looks like a great start; some general comments -- the widget backend shouldn't be called "cairo", but "headless" or "offscreen".  The define used in widget/src/Makefile.in shouldn't be MOZ_ENABLE_CAIRO.  Also, the glib/gobject dependency should be removed because this is something that's would be very useful cross-platform, and there's no reason to drag in glib/gobject.
I think "offscreen" is the best of those suggestions and most accurately reflects what the backend does.
What are you using glib for?  A mainloop?
I think we should just land this on trunk. There is no need for a separate branch or repo. It's all code that's not going to be built by any shipping product.
(In reply to comment #14)
> I think we should just land this on trunk. There is no need for a separate
> branch or repo. It's all code that's not going to be built by any shipping
> product.

I agree, but not until at least the build issues are resolved and things like naming and stuff are fixed.  De-glibification and whatever else can absolutely happen after.
Sorry yes, I mean "when it's ready (not perfect), we should land it on trunk, not a branch".
Vlad, can you be specific about what you want to see before you would want it on trunk?
Pretty much exactly what I said in comment #11, minus the glib bits.
I agree with everything said in comment #11, I realised after I'd got it working that using GLib was unnecessary, and if anything, more of a hinderance. The GLib mainloop and signals are used, but these can be replaced by callbacks and an 'iterate' function, I suppose?

De-glibification will take a while though and there are other things I need to get working first. It would not just be me hacking on it, Geoff Gustafson is working on getting plug-ins working and it's possible that other people might want to contribute.

I'll go about fixing the naming issues as soon as I can; Development can be tracked in our public git repository; http://git.o-hand.com/ , in the 'mozilla-headless' repo.
How do you plan to support plugins?
(In reply to comment #19)
> I agree with everything said in comment #11, I realised after I'd got it
> working that using GLib was unnecessary, and if anything, more of a hinderance.
> The GLib mainloop and signals are used, but these can be replaced by callbacks
> and an 'iterate' function, I suppose?

Yeah, callbacks/iterate would be perfect.

> De-glibification will take a while though and there are other things I need to
> get working first. It would not just be me hacking on it, Geoff Gustafson is
> working on getting plug-ins working and it's possible that other people might
> want to contribute.

Yep, no hurry on the de-glib stuff; that shouldn't block this from getting in.  I think the naming issues are the only things that should get fixed to avoid build weirdness and needing to rename later.

Thanks for doing this!  I had wanted to do something similar for a while but haven't had the time, glad to see someone beat me to it :)  We've gotten a number of requests from people wanting to do this and haven't had a good answer for them.
The attached patch has the changes discussed (except the de-glibification), and also has a few bug-fixes and features. If this is going into trunk, it'd be great to sort out how I'm going to have access to that.
Attachment #350161 - Attachment is obsolete: true
Assignee: nobody → chris
(In reply to comment #22)
> Created an attachment (id=352100) [details]
> Updated patch (applies against default:390be3dc17d8)
> 
> The attached patch has the changes discussed (except the de-glibification), and
> also has a few bug-fixes and features. If this is going into trunk, it'd be
> great to sort out how I'm going to have access to that.

Yep, I'm working on figuring out the access bits -- worst case it might be a cloned repo hosted on hg.mozilla.org with someone else doing periodic merges back in to the trunk, like we do with the tracemonkey work.  That shouldn't be too bad as the build/ifdef/etc. changes would mostly be in the initial patch.

I haven't had a chance to look at the patch yet in detail though, but one question came up form my quick skim; why the use of another moz_drawing_surface class/object instead of just keeping everything directly on the nsWindow impl?  Is it just to have a glib-style object for the glib signal hooks?  And if so, when we de-glibified, would you still want to keep that separate object?

I mainly ask because since this is essentially a brand new widget backend, it would be nice to have the code be able to serve as a sort of barebones reference for someone doing a new backend; the gtk backend is pretty crufty, so it's probably not the best model to follow.  I realize that this isn't really a goal of the work you're doing though, so we can probably just get there incrementally :)
(In reply to comment #23)
> Yep, I'm working on figuring out the access bits -- worst case it might be a
> cloned repo hosted on hg.mozilla.org with someone else doing periodic merges
> back in to the trunk, like we do with the tracemonkey work.  That shouldn't be
> too bad as the build/ifdef/etc. changes would mostly be in the initial patch.

That's fine, as long as there's a repository that we can fetch from with the latest work and that I can commit to unhindered, that's enough. It'd be great if it automatically merged too, though if the initial patch is in trunk, this shouldn't be a problem anyway, like you say.

> I haven't had a chance to look at the patch yet in detail though, but one
> question came up form my quick skim; why the use of another moz_drawing_surface
> class/object instead of just keeping everything directly on the nsWindow impl? 
> Is it just to have a glib-style object for the glib signal hooks?  And if so,
> when we de-glibified, would you still want to keep that separate object?

I was thinking of this myself too; There does need to be a separate object for the embedding API to work as it does currently (the root window doesn't have an associated nsWindow), but I imagine some of the logic could be merged back into nsWindow. Having the separate drawing-area object does make nsWindow.cpp much more readable though.

> I mainly ask because since this is essentially a brand new widget backend, it
> would be nice to have the code be able to serve as a sort of barebones
> reference for someone doing a new backend; the gtk backend is pretty crufty, so
> it's probably not the best model to follow.  I realize that this isn't really a
> goal of the work you're doing though, so we can probably just get there
> incrementally :)

This would be nice, I tried to simplify as much as I could (the nsWindow is mostly derived from the Qt backend rather than the Gtk one), but there's still a ways to go. It could be better documented too...

De-glibification is low on my list of priorities though really, I'd very much like to have 'native' scrolling support in first. This requires changes in mozilla though and I could really use some help on this. I've put this on the back-burner until it's merged.
Hey, has there been any progress on this? Things have gone a bit quiet...
Looking back into the bug it's not clear what the next step is here.  Get reviews and get it into trunk?  Set up another repo?  I think it's the former, but if so we should start that process.  Is the patch in here the most recent?  Vlad had some high-level comments, but I think that you were open to fixing them.
This is the most recent patch, yes (I've purposefully busied myself with other things as I thought it was in the process of being pushed).

As I said in the attachment comment, the current patch has the issues that people pointed out fixed, other than the de-glibification. It's by no means final code, but it doesn't affect (and mostly doesn't touch) any of the current code and it's a good enough starting point.

Going from comment #15, I'd have thought it'd be ok to commit now?
Attachment #352100 - Flags: superreview?(vladimir)
Attachment #352100 - Flags: review?(roc)
Posted review queries.
Sorry, I've been trying to figure out the process for setting up the incubator-style repo -- this'll be only the second one we've done, and it's not quite clear how to do it. :)  Being right around the holidays isn't helping, but I'll hopefully have some info for you in another few days.

One thing that would be helpful for reviews is if you could split out the changes to existing files (build etc.) from the new files (the actual widget impl, the embedding impl, etc.); the changes to existing files is the bit that really needs the detailed review.
I thought we agreed we want this in mozilla-central, not a separate repo.
Getting it into an incubator repo is the easiest way to proceed for now -- the initial patch should go into m-c, and we can do frequent merges tracemonkey-style.  I'll volunteer to be on hook for doing offscreen -> m-c merges often.
+#elif defined(MOZ_WIDGET_GTK2) || defined(MOZ_WIDGET_HEADLESS)
 #include "nsGNOMEShellService.h"
 #endif

-#elif defined(MOZ_WIDGET_GTK2)
+#elif defined(MOZ_WIDGET_GTK2) || defined(MOZ_WIDGET_HEADLESS)
 NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(nsGNOMEShellService, Init)
 #endif

I think we need some kind of MOZ_GNOME macro, or maybe MOZ_XDG?

But headless doesn't actually need an nsIShellService does it? I presume you don't want a headless browser to be the user's default browser :-).

That leads me to ask, is the goal here limited to just getting embedding or custom XUL apps to run headless? Or is the goal actually a full headless port of Firefox?

What is HeadlessEventListener for? If you're going to synthesize input events, why do you need to be notified of those events?

HeadlessPrompter confuses me. Why do you want your headless browser to pop up GTK dialogs? I feel like I'm missing something big here.

Any chance we could get a small demo or better still, automated-test app for your headless API?

Small-potatoes drive-by comments:

+               (mChromeMask & nsIWebBrowserChrome::CHROME_SCROLLBARS ?
+                PR_TRUE : PR_FALSE);
(mChromeMask & nsIWebBrowserChrome::CHROME_SCROLLBARS) != 0

+    PRBool       mCheckValue;
+    PRBool       mConfirmResult;
As a matter of style we prefer to use PRPackedBools here (1 byte rather than 4) and group them together to avoid alignment space wastage.

Skimmed the embedding stuff. I'll look at the gfx and widget parts next.
More small-potatoes comments:

+  MOZ_HEADLESS_FLAG_DEFAULTCHROME = 1U,
+  MOZ_HEADLESS_FLAG_WINDOWBORDERSON = 2U,
+  MOZ_HEADLESS_FLAG_WINDOWCLOSEON = 4U,
+  MOZ_HEADLESS_FLAG_WINDOWRESIZEON = 8U,
+  MOZ_HEADLESS_FLAG_MENUBARON = 16U,
+  MOZ_HEADLESS_FLAG_TOOLBARON = 32U,
+  MOZ_HEADLESS_FLAG_LOCATIONBARON = 64U,
+  MOZ_HEADLESS_FLAG_STATUSBARON = 128U,
+  MOZ_HEADLESS_FLAG_PERSONALTOOLBARON = 256U,
+  MOZ_HEADLESS_FLAG_SCROLLBARSON = 512U,
+  MOZ_HEADLESS_FLAG_TITLEBARON = 1024U,
+  MOZ_HEADLESS_FLAG_EXTRACHROMEON = 2048U,
+  MOZ_HEADLESS_FLAG_ALLCHROME = 4094U,
+  MOZ_HEADLESS_FLAG_WINDOWRAISED = 33554432U,
+  MOZ_HEADLESS_FLAG_WINDOWLOWERED = 67108864U,
+  MOZ_HEADLESS_FLAG_CENTERSCREEN = 134217728U,
+  MOZ_HEADLESS_FLAG_DEPENDENT = 268435456U,
+  MOZ_HEADLESS_FLAG_MODAL = 536870912U,
+  MOZ_HEADLESS_FLAG_OPENASDIALOG = 1073741824U,
+  MOZ_HEADLESS_FLAG_OPENASCHROME = 2147483648U 

I always feel that constants like these are better written in hex format.

+  nsCOMPtr <nsIDOMMouseEvent> mouseEvent;

No space before the <

+    case eSystemFont_Menu:         // css2
+    case eSystemFont_PullDownMenu: // css3
+        *aFontName = mMenuFontName;
+        *aFontStyle = mMenuFontStyle;
+        break;

IMHO you'd be beter off just doing aFontName->AssignLiteral("sans-serif") and *aFontStyle = mDefaultFontStyle ... the variables aren't buying you anything.

+CPPSRCS +=      gfxPlatformHeadless.cpp
+CPPSRCS +=      gfxPangoFonts.cpp
+#CPPSRCS +=      gfxPDFSurface.cpp gfxPSSurface.cpp
+CPPSRCS +=      gfxFontconfigUtils.cpp
+CPPSRCS +=      nsUnicodeRange.cpp

The usual syntax would be better:
CPPSRCS += 	gfxPlatformHeadless.cpp \
		gfxPangoFonts.cpp \
		gfxFontconfigUtils.cpp \
		nsUnicodeRange.cpp \
		$(NULL)

I wonder why nsUnicodeRange is defined per-platform since every platform seems to use it...

+    gfxASurface *newSurface = nsnull;
+
+    newSurface = new gfxImageSurface(size, imageFormat);

Why not do this in one statement?

+    NS_ADDREF(newSurface);

NS_IF_ADDREF in case of OOM

 ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2)
 OSHELPER	+= nsGNOMERegistry.cpp
+OSHELPER  += nsMIMEInfoUnix.cpp
+endif
+
+ifeq ($(MOZ_WIDGET_TOOLKIT),headless)
+OSHELPER  += nsGNOMERegistry.cpp
 OSHELPER  += nsMIMEInfoUnix.cpp
 endif

This is another place where we kinda need MOZ_GNOME or something similar. We definitely don't need to sort that out before this lands though.

+nsScreenHeadless :: GetPixelDepth(PRInt32 *aPixelDepth)
+{
+  *aPixelDepth = 32;

Should be 24.

+  nsRect mRect; // in pixels, not twips
+  nsRect mAvailRect; // in pixels, not twips
+  mAvailRect = mRect = nsRect(0, 0, 800, 600);

Make these nsIntRect.

nsScreenManagerHeadless could use some trimming --- it has some code to support multiple screens, which presumably isn't part of the plan for the headless backend :-).

It would be nice if this WAV-playing code could just use our nsWaveDecoder and nsAudioStream (based on libsydney-audio). Food for thought...

In nsWindow::ScrollWidgets:
+    // Invalidate the newly exposed area
Is this actually needed? Gecko does this itself at a higher level.

+    moz_drawing_area_redraw (mDrawingarea, aSynchronous ? TRUE : FALSE);

Why not just pass aSynchronous? PRBools are guaranteed to be either 0 or 1.

I haven't seen anything here that should block checkin IMHO.
> This is another place where we kinda need MOZ_GNOME or something similar. We
> definitely don't need to sort that out before this lands though.

Yeah, we have a big mess between platforms, widget systems, graphics backends, etc.  Would be nice to figure that out (definitely not for this bug though), but it's going to be a pretty big ugly task.  Mark Steele did a port that lets you build with native gtk on OSX, and had to deal with a bunch of this -- mac especially is in bad shape (XP_MAC vs. XP_MACOSX vs. XP_UNIX vs. MOZ_X11 etc.).

I started tracking down people to get the incubator repo set up, but it got bogged down over the holidays.  Will restart next week.
(In reply to comment #34)
> > This is another place where we kinda need MOZ_GNOME or something similar. We
> > definitely don't need to sort that out before this lands though.
> 
> Yeah, we have a big mess between platforms, widget systems, graphics backends,
> etc.  Would be nice to figure that out (definitely not for this bug though),
> but it's going to be a pretty big ugly task.  Mark Steele did a port that lets
> you build with native gtk on OSX, and had to deal with a bunch of this -- mac
> especially is in bad shape (XP_MAC vs. XP_MACOSX vs. XP_UNIX vs. MOZ_X11 etc.).
> 
> I started tracking down people to get the incubator repo set up, but it got
> bogged down over the holidays.  Will restart next week.

As we no longer support non OSX MAC I would have thought one of those would go away, also with gnome deprecating libgnome, libgnomeui and gnome_vfs and moving the functions to equivalent gtk functions (see bugs 467168 and 402892) there's probably no real need for a specific GNOME define but just one for gtk which would work across all platforms (windows/mac/unix).
(In reply to comment #32)
> +#elif defined(MOZ_WIDGET_GTK2) || defined(MOZ_WIDGET_HEADLESS)
>  #include "nsGNOMEShellService.h"
>  #endif
> 
> -#elif defined(MOZ_WIDGET_GTK2)
> +#elif defined(MOZ_WIDGET_GTK2) || defined(MOZ_WIDGET_HEADLESS)
>  NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(nsGNOMEShellService, Init)
>  #endif
> 
> I think we need some kind of MOZ_GNOME macro, or maybe MOZ_XDG?
> 
> But headless doesn't actually need an nsIShellService does it? I presume you
> don't want a headless browser to be the user's default browser :-).

This is really just from having done lots of copy/paste from the gtk backend and initially not really knowing how I was going about things. Theoretically, you could have a headless server that a client could connect to though? Not really a goal at the moment though.

> That leads me to ask, is the goal here limited to just getting embedding or
> custom XUL apps to run headless? Or is the goal actually a full headless port
> of Firefox?

I think the last paragraph answers this :) A headless Firefox would be pretty cool though...

> What is HeadlessEventListener for? If you're going to synthesize input events,
> why do you need to be notified of those events?

I think this is legacy from some experiments or copy/paste? I haven't touched the code in a couple of weeks now, so I'm not entirely sure.

> HeadlessPrompter confuses me. Why do you want your headless browser to pop up
> GTK dialogs? I feel like I'm missing something big here.

I don't, but I want (eventually) the headless backend to be able to signal that it wants to pop up a dialog and then the embedder can do that how they want.

> Any chance we could get a small demo or better still, automated-test app for
> your headless API?

There's a test app in git://git.o-hand.com/clutter-mozembed.git (requires Clutter). Note that the browser demo in there requires the assets from git://git.clutter-project.org/toys.git mallums-magic-browser (http://git.clutter-project.org/cgit.cgi?url=toys/tree/mallums-magic-browser), though it will run without.

> Small-potatoes drive-by comments:
> 
> +               (mChromeMask & nsIWebBrowserChrome::CHROME_SCROLLBARS ?
> +                PR_TRUE : PR_FALSE);
> (mChromeMask & nsIWebBrowserChrome::CHROME_SCROLLBARS) != 0
> 
> +    PRBool       mCheckValue;
> +    PRBool       mConfirmResult;
> As a matter of style we prefer to use PRPackedBools here (1 byte rather than 4)
> and group them together to avoid alignment space wastage.

Okidokes - I think this is just copy/paste from the gtk embedding API, so probably needs fixing there too.

> Skimmed the embedding stuff. I'll look at the gfx and widget parts next.
(In reply to comment #33)
> More small-potatoes comments:
> 
> +  MOZ_HEADLESS_FLAG_DEFAULTCHROME = 1U,
> +  MOZ_HEADLESS_FLAG_WINDOWBORDERSON = 2U,
> +  MOZ_HEADLESS_FLAG_WINDOWCLOSEON = 4U,
> +  MOZ_HEADLESS_FLAG_WINDOWRESIZEON = 8U,
> +  MOZ_HEADLESS_FLAG_MENUBARON = 16U,
> +  MOZ_HEADLESS_FLAG_TOOLBARON = 32U,
> +  MOZ_HEADLESS_FLAG_LOCATIONBARON = 64U,
> +  MOZ_HEADLESS_FLAG_STATUSBARON = 128U,
> +  MOZ_HEADLESS_FLAG_PERSONALTOOLBARON = 256U,
> +  MOZ_HEADLESS_FLAG_SCROLLBARSON = 512U,
> +  MOZ_HEADLESS_FLAG_TITLEBARON = 1024U,
> +  MOZ_HEADLESS_FLAG_EXTRACHROMEON = 2048U,
> +  MOZ_HEADLESS_FLAG_ALLCHROME = 4094U,
> +  MOZ_HEADLESS_FLAG_WINDOWRAISED = 33554432U,
> +  MOZ_HEADLESS_FLAG_WINDOWLOWERED = 67108864U,
> +  MOZ_HEADLESS_FLAG_CENTERSCREEN = 134217728U,
> +  MOZ_HEADLESS_FLAG_DEPENDENT = 268435456U,
> +  MOZ_HEADLESS_FLAG_MODAL = 536870912U,
> +  MOZ_HEADLESS_FLAG_OPENASDIALOG = 1073741824U,
> +  MOZ_HEADLESS_FLAG_OPENASCHROME = 2147483648U 
> 
> I always feel that constants like these are better written in hex format.

Copy/paste from gtk embedding, I think.

> +  nsCOMPtr <nsIDOMMouseEvent> mouseEvent;
> 
> No space before the <

Okidokes

> +    case eSystemFont_Menu:         // css2
> +    case eSystemFont_PullDownMenu: // css3
> +        *aFontName = mMenuFontName;
> +        *aFontStyle = mMenuFontStyle;
> +        break;
> 
> IMHO you'd be beter off just doing aFontName->AssignLiteral("sans-serif") and
> *aFontStyle = mDefaultFontStyle ... the variables aren't buying you anything.

Eventually, would want these to be able to be specified by the embedding client.

> +CPPSRCS +=      gfxPlatformHeadless.cpp
> +CPPSRCS +=      gfxPangoFonts.cpp
> +#CPPSRCS +=      gfxPDFSurface.cpp gfxPSSurface.cpp
> +CPPSRCS +=      gfxFontconfigUtils.cpp
> +CPPSRCS +=      nsUnicodeRange.cpp
> 
> The usual syntax would be better:
> CPPSRCS +=     gfxPlatformHeadless.cpp \
>         gfxPangoFonts.cpp \
>         gfxFontconfigUtils.cpp \
>         nsUnicodeRange.cpp \
>         $(NULL)

Sure, again probably a copy/pasting thing so may need fixing elsewhere too.

> I wonder why nsUnicodeRange is defined per-platform since every platform seems
> to use it...
> 
> +    gfxASurface *newSurface = nsnull;
> +
> +    newSurface = new gfxImageSurface(size, imageFormat);
> 
> Why not do this in one statement?

Legacy from when I did something slightly different there I think :)

> +    NS_ADDREF(newSurface);
> 
> NS_IF_ADDREF in case of OOM

Okidokes. There are probably quite a few other cases where OOM isn't handled...

>  ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2)
>  OSHELPER    += nsGNOMERegistry.cpp
> +OSHELPER  += nsMIMEInfoUnix.cpp
> +endif
> +
> +ifeq ($(MOZ_WIDGET_TOOLKIT),headless)
> +OSHELPER  += nsGNOMERegistry.cpp
>  OSHELPER  += nsMIMEInfoUnix.cpp
>  endif
> 
> This is another place where we kinda need MOZ_GNOME or something similar. We
> definitely don't need to sort that out before this lands though.
> 
> +nsScreenHeadless :: GetPixelDepth(PRInt32 *aPixelDepth)
> +{
> +  *aPixelDepth = 32;
> 
> Should be 24.

I think I made it 32 as I wasn't sure if pages could have semi-transparent backgrounds? Also, it aided with debugging at one point, but that's no longer an issue. Assuming my first statement is invalid, will change.

> +  nsRect mRect; // in pixels, not twips
> +  nsRect mAvailRect; // in pixels, not twips
> +  mAvailRect = mRect = nsRect(0, 0, 800, 600);
> 
> Make these nsIntRect.

Sure

> nsScreenManagerHeadless could use some trimming --- it has some code to support
> multiple screens, which presumably isn't part of the plan for the headless
> backend :-).

Probably not, no :) I'll need to have a think though, this again might be a case where it may be useful to have some embedding API to handle cases with multiple screens nicely.

> It would be nice if this WAV-playing code could just use our nsWaveDecoder and
> nsAudioStream (based on libsydney-audio). Food for thought...

Copied straight from the GTK back-end. I'd rather not use esound either, so if there's some example code somewhere...

> In nsWindow::ScrollWidgets:
> +    // Invalidate the newly exposed area
> Is this actually needed? Gecko does this itself at a higher level.

I think it is, if anything I think there are some missing synthetic expose events in places.

> +    moz_drawing_area_redraw (mDrawingarea, aSynchronous ? TRUE : FALSE);
> 
> Why not just pass aSynchronous? PRBools are guaranteed to be either 0 or 1.

Just being extra careful/lack of knowledge. Over-cautious I suppose, will change.

> I haven't seen anything here that should block checkin IMHO.

Excellent :) Keep me informed how things are going and thanks for the review!
(In reply to comment #36)
> (In reply to comment #32)
> > +#elif defined(MOZ_WIDGET_GTK2) || defined(MOZ_WIDGET_HEADLESS)
> >  #include "nsGNOMEShellService.h"
> >  #endif
> > 
> > -#elif defined(MOZ_WIDGET_GTK2)
> > +#elif defined(MOZ_WIDGET_GTK2) || defined(MOZ_WIDGET_HEADLESS)
> >  NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(nsGNOMEShellService, Init)
> >  #endif
> > 
> > I think we need some kind of MOZ_GNOME macro, or maybe MOZ_XDG?
> > 
> > But headless doesn't actually need an nsIShellService does it? I presume you
> > don't want a headless browser to be the user's default browser :-).
> 
> This is really just from having done lots of copy/paste from the gtk backend
> and initially not really knowing how I was going about things. Theoretically,
> you could have a headless server that a client could connect to though? Not
> really a goal at the moment though.

I'd just take it out. Firefox uses this to do things like pop up a dialog asking if you want to make it the default browser. You just don't need this functionality :-).

> > That leads me to ask, is the goal here limited to just getting embedding or
> > custom XUL apps to run headless? Or is the goal actually a full headless port
> > of Firefox?
> 
> I think the last paragraph answers this :) A headless Firefox would be pretty
> cool though...

OK

(In reply to comment #37)
> > +nsScreenHeadless :: GetPixelDepth(PRInt32 *aPixelDepth)
> > +{
> > +  *aPixelDepth = 32;
> > 
> > Should be 24.
> 
> I think I made it 32 as I wasn't sure if pages could have semi-transparent
> backgrounds? Also, it aided with debugging at one point, but that's no longer
> an issue. Assuming my first statement is invalid, will change.

Web pages normally aren't allowed to have transparent backgrounds, although with a bit of work you can make that possible if you want. But this doesn't affect that; this is a number that's actually reported to Web content via some DOM APIs, and it only cares about color channels, not alpha.

> > It would be nice if this WAV-playing code could just use our nsWaveDecoder and
> > nsAudioStream (based on libsydney-audio). Food for thought...
> 
> Copied straight from the GTK back-end. I'd rather not use esound either, so if
> there's some example code somewhere...

There isn't unfortunately. This nsSound stuff predates our video/audio work by many years.

> > In nsWindow::ScrollWidgets:
> > +    // Invalidate the newly exposed area
> > Is this actually needed? Gecko does this itself at a higher level.
> 
> I think it is, if anything I think there are some missing synthetic expose
> events in places.

Not sure what's going on there but it doesn't matter much at this stage.

> Excellent :) Keep me informed how things are going and thanks for the review!

No worries. Vlad, I think we should go ahead and get repos set up and stuff. When we get a new patch from Chris I think we can land.
So, according to bug #470278, approval has been granted for the incubator repository; do I need to update this patch to apply against tip (assuming it doesn't now), or is it ok as it is for now?
I think Vlad can just land everything. Your patch shouldn't require much updating, if any. But if you want to update it to address any of the issues I mentioned above, now would be a good time.
Ok, excellent, if you could hold off and I'll fix a couple of things, update and reattach a new patch in the next couple of days.
Sounds good -- in the meantime, can you do the stuff in http://www.mozilla.org/hacking/incubator-repository.html#commit to get a hg account for yourself?  Cc me/roc on the bug and we'll sponsor.
Done - I take it the thing about faxing things off to places is out-of-date...? Either way, filed as bug #473532 and Cc'd you and roc on it.
This is an updated patch with a couple of the things from the comments fixed (not all of them). No new features (so the prior review should still be ok?)
Attachment #352100 - Attachment is obsolete: true
Attachment #352100 - Flags: superreview?(vladimir)
Attachment #352100 - Flags: review?(roc)
Product: Core → Core Graveyard
Component: GFX → Widget
Product: Core Graveyard → Core
QA Contact: general → general
Latest patch seems to apply cleanly to current trunk; just sent it off to try server to make sure there aren't any weird cross-platform compile issues with the current configs.  If everything's ok, I'll check the patch in the offscreen incubator repo and then merge it into m-c.

For the account, you do need to fax in the committers' agreement form -- I think reed's waiting on that in the other bug.
I'd be surprised if that patch builds... I think it needs to be updated for the nsRect/nsIntRect separation.
Well, the patch builds for our current builds -- no guarantees whether it would build with the offscreen backend.  I'd rather get it checked in even in that non-building state and fix that up than to have Chris keep trying to maintain the huge patch.  Looks like it was clean on the tinderbox everywhere.
Hate to be a pain, but if this could be the patch that gets applied, that'd be fantastic. This patch fixes input issues and fixes the 'scroll' signal so that embedders can cancel the memory copy that happens when scrolling and do their own thing instead.
Attachment #357157 - Attachment is obsolete: true
Even better, here's the patch updated to apply and build/work against tip.
Attachment #359241 - Attachment is obsolete: true
Yep, no problem -- the trunk's actually closed for another day or so while we try to get b3 stuff landed, but I'll merge this in right after.

Is there a test anywhere?  Maybe something like a command-line tool that you could specify a URL, width & height, and an output png to write to.
Just to note, the code is up in the 'headless' branch of the offscreen incubator repository now; http://hg.mozilla.org/incubator/offscreen
What is it merged to?  A branch or mozilla-central?
Just a heads-up, bug 431634 is going to require work to keep this port up to date.

It would be nice to have a version of this landed in mozilla-central so that people doing wholesale changes to widget interfaces can at least attempt to keep headless building.
(In reply to comment #54)
> What is it merged to?  A branch or mozilla-central?

mozilla-central
(In reply to comment #55)
> Just a heads-up, bug 431634 is going to require work to keep this port up to
> date.

Sure, I've no problem keeping this in sync. It'd be good if someone could poke me on IRC if they know big changes are going in (or if there's some way of finding this out without polling)

> It would be nice to have a version of this landed in mozilla-central so that
> people doing wholesale changes to widget interfaces can at least attempt to
> keep headless building.

Agreed :)
I've just merged mozilla-central into headless again (see http://hg.mozilla.org/incubator/offscreen) - If someone could confirm that they'll merge with the code that's there and not the attached patch, that'd be great :)
Comment on attachment 359262 [details] [diff] [review]
Updated patch (applies against default:93bf83df396b)

This patch is obsolete - the latest code is in an incubator repository: http://hg.mozilla.org/incubator/offscreen
Attachment #359262 - Attachment is obsolete: true
hg clone  http://hg.mozilla.org/incubator/offscreen/
-or-
hg clone  http://hg.mozilla.org/incubator/offscreen/file/fe09b4a17023

both skip cloning the directory /embedding/browser/headless/ in the resulting code tree the headless directory is visible while browsing the repo but appears not to be clone-able. 

tested on  win32 and Linux 64 bit environments, both have the same problem, no headless directory.
To checkout,

hg clone http://hg.mozilla.org/incubator/offscreen
hg checkout headless

(or headless-plugins if you want to experiment with the plugin support)
oh.. this is really cool...
in current microb-browser we already using almost "headless" implementation... it is current implementation with widget/gtk, but that widget mostly not active, expose and update events are disabled, and widget hidden inside some vbox...

All Xlib surfaces creation replaced to gfxImageSurface, plugin rendering changed to local image rendering (bug 477456), and all painting done via RenderDocument API...
I was thinking about killing nsIWidget realization (initialize webBrowser without nsIWidget), but there are still unsolved problem with key events hacks and tweaks which is implemented inside gtk/nsWindow.cpp...

Also in our case we are pre-rendering offscreen image on UI side (and using ignoreScrollFrame flags a lot, similar to Fennec browser)...
Maybe in headless implementation we should provide some implementation for working in out-scroll-frame mode...
hi there, 
I cloned and checked out the headless branch, I was able to build it without errors but whatever way I try I can't seem to use it. 

since I haven't touched C in a veeeery long time, I first tried the xulrunner but it died with the following :

JavaScript error: , line 0: uncaught exception: [Exception... "Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsIDOMChromeWindow.getAttention]" nsresult: "0x80004001 (NS_ERROR_NOT_IMPLEMENTED)" location: "JS frame :: chrome://global/content/commonDialog.js :: commonDialogOnLoad :: line 195" data: no]

you can get my xul app there (tar.gz) : http://conceptin.free.fr/mozilla/offbrwse_xul/offbrowse.tar.gz

I then tried compiling the sample moz_headless_screenshot.c from chris lord, after quite a bit of error and trial I got it to compile but it dies on me with a segfault. the whole output of the run can be found there : 
http://conceptin.free.fr/mozilla/headless_embedded/error.txt

my command line was: ./offbrowse http://www.google.com 800 600 test.png
I think the sample bitrotted a bit. You'll need to add moz_headless_push_startup () / moz_headless_pop_startup () calls.

You'll also need to set the MOZILLA_FIVE_HOME environment variable pointing to the directory containing libxul.so ($prefix/lib/xulrunner-1.9.2a1pre/) when running the sample.
I replied to this on twitter, but I'll add the information as it may be useful for others;

The embedding API requires that you either export MOZILLA_FIVE_HOME, or set the mozilla data directory with moz_headless_set_path (). This directory should be something along the lines of /usr/local/lib/xulrunner-1.9.2a1pre - If you have things setup correctly, you can get the path from pkg-config like this:

echo `pkg-config --variable=prefix mozilla-headless`/lib/xulrunner-`pkg-config --modversion mozilla-headless`

Regarding this, I followed how the gtk embedding API works, but why this path isn't set correctly by default, I've no idea. I may change this (and you'll note that clutter-mozembed does this for you), but I'd like to find out the reasoning behind the behaviour in the gtk embedding API first.

As to xulrunner not working, I don't know why that is - the error looks like it'll be something simple and I'll take a look as soon as I have time.
(In reply to comment #64)
> Created an attachment (id=374833) [details]
> patch for the screenshot sample
> 
> I think the sample bitrotted a bit. You'll need to add
> moz_headless_push_startup () / moz_headless_pop_startup () calls.
> 
> You'll also need to set the MOZILLA_FIVE_HOME environment variable pointing to
> the directory containing libxul.so ($prefix/lib/xulrunner-1.9.2a1pre/) when
> running the sample.

You shouldn't need to call push_startup/pop_startup, these are called when you create a new window.
er, yeah it works without these calls. I think I tried to add them at the same time I discovered I had to set MOZILLA_FIVE_HOME and wrongly assumed these calls solved the issue.

Sorry for the confusion.
Hi, All
I cloned offscreen and checkout headless and headless-plugins. Then try to make build offscreen with Chris Lord's .mozconfig attached in this thread. But I met the following warning:
"Cannot build gnomevfs without required libraries. Removing gnomevfs from MOZ_EXTENSIONS.".
I checked my host and gnome-vfs development files were installed. 
How can I fix it?
(In reply to comment #62)
> oh.. this is really cool...
> in current microb-browser we already using almost "headless" implementation...
> it is current implementation with widget/gtk, but that widget mostly not
> active, expose and update events are disabled, and widget hidden inside some
> vbox...
> 
> All Xlib surfaces creation replaced to gfxImageSurface, plugin rendering
> changed to local image rendering (bug 477456), and all painting done via
> RenderDocument API...
> I was thinking about killing nsIWidget realization (initialize webBrowser
> without nsIWidget), but there are still unsolved problem with key events hacks
> and tweaks which is implemented inside gtk/nsWindow.cpp...
> 
> Also in our case we are pre-rendering offscreen image on UI side (and using
> ignoreScrollFrame flags a lot, similar to Fennec browser)...
> Maybe in headless implementation we should provide some implementation for
> working in out-scroll-frame mode...

Thanks for the hint on this, I'm going to take a look at it to allow for backends to buffer outside of the scroll region to implement smooth scrolling.

Also, just to add a note to this bug, the 'headless-plugins' branch is deprecated now, plugins support is in the main 'headless' branch.
Chris, have you spent any time looking at the out of process plugin stuff yet?  Just wondering since it seems right up your alley right now.
(In reply to comment #70)
> Chris, have you spent any time looking at the out of process plugin stuff yet? 
> Just wondering since it seems right up your alley right now.

We had a discussion about it, and it seems we're taking quite different paths in how we go about it. Where as we put the entirety of Mozilla in a separate process and implement key interfaces to allow multiple processes to work, the current Mozilla plan is to just separate the rendering processes into threads, but still have a single chrome process through which all communication happens; including network IO.

Of course, the way you guys will do it will mean less memory use (possibly at the cost of stability? I don't know...) and will be easier in the long run, but it still limits accessibility of Mozilla data. Unless the chrome process implemented some kind of access API and was able to run as a daemon (which isn't an unreasonable thing to do), you wouldn't be able to integrate it with the rest of the system like we currently do in Moblin (or rather, like we have plans to do and currently have only partially implemented ;))

I'm not entirely sure what the best way for either of us is to take advantage of each other's work. I imagine as interfaces are simplified/split up to make your work easier, it'll probably help us too, but I'm not sure if anything we're doing (beyond the headless back-end of course) will help you... I'm open to suggestions though.
I think that breaking chrome + content into their own processes is a first step.  From there they will see if moving to a per-window or per-site model for processes is reasonable or not.

But I was mostly talking about plugins (aka flash) as separate processes, not the entire browser.

You might want to connect with those folks anyway - I'm sure you could share what you're up to.
A headless mozilla is exactly what I'm looking for.  I just attempted to build this on windows, and ran into problems with dependencies on gtk+-unix-print-2.0.  I'm afraid most of the implementation details you have been discussing here are quite above my head, but I wanted to ask if it should even be possible to build this on windows until the "de-glibification" happens?

What I have done so far:

hg clone http://hg.mozilla.org/incubator/offscreen
hg checkout headless

Created .mozconfig file based on the attachment to this bug

make -f client.mk build

Failed here because of dependency on pkg-config and I had not installed the gtk+ libs.  Installed the libs from http://ftp.gnome.org/pub/gnome/binaries/win32/gtk+/2.16/gtk+-bundle_2.16.4-20090708_win32.zip, and placed the bin directory in path

make -f client.mk build

Got further, but failed with dependency on gtk+-unix-print-2.0 and gdk-x11-2.0.
Are plugins expected to re(In reply to comment #52)
> I wrote a quick app like this for my talk in FOSDEM;
> 
> App: http://chrislord.net/files/moz-headless-screenshot.c

Ok following this thread I have this test application working and I'm quite impressed. My question is regarding plugin support, currently anything with flash on it renders as a transparent area using the test-app. Is that expected?
Attached image Flash test page — 

    
    

    
  
Sorry for not updating this bug in so long, other commitments have kept me busy.

I now have some time to dedicate to getting this merged, if possible. As a first step, I've merged again with mozilla-central and tidied up/fixed a few things. The changes don't really touch anything outside of the new widget/gfx/embedding files (and a few defines in layout for plugin support).

For convenience, I'll attach the latest diff to this bug report, but as always, the very latest is in the offscreen incubator repository in the 'headless' branch.

It'd be great if I could get some review and comment on getting this merged into mozilla-central (and thanks for everyone's continued help and encouragement).

    
    

    
  
In case anyone is interested, IMVU just implemented Gecko rendering on top of 3D windows (Direct3D, OpenGL, and Pixomatic).  We had to apply two patches to Mozilla.  One exposes an API to render a region of the document to memory (including stride) and the other notifies us of dirty rectangles when they change so we can efficiently update the overlay textures.

Eventually, we plan to use Gecko for all HUD rendering.

Unfortunately, the guy who implemented this is out for all of November.  I'll have him attach the patches and a technical description when he returns.

    
    

    
  
Karl might be able to help with reviews here. People are a little busy over the next couple of weeks nailing down Firefox 3.6 issues, but we should be good after that.

Chad, good to know. Thanks!

You guys might be interested in following our plans for accelerated rendering. There's a wiki page here:
https://wiki.mozilla.org/Gecko:Layers
I guess I should post to dev.platform about it.

    
    

    
  
BTW, Chad, it sounds like you didn't use the headless branch here. Could you have? Could you comment on how it well it would suit your needs?

    
    

    
  
Thanks for the heads up on the accelerated rendering front.

Unfortunately, I'll have to wait until the primary developer on the HTML overlay project comes back from vacation before I can comment on the suitability of the headless branch.  But if I remember his comments correctly, to get the basic implementation running, we just needed two capabilities:

Given an nsIPresShell, we need to render a rectangular region of the document to a 32-bit RGBA pixmap (preferably in COLORREF byte order) with an arbitrary stride.

In addition, we need a notification when rectangles become dirty so we can lazily update the texture before the next render.

    
    

    
  
Sorry for the e-mail spam, but I just remembered a couple other important features:

It helps a lot if rendering a document to an in-memory buffer preserves alpha (whether premultiplied or not).  Otherwise, like with our Flash overlay system, we have to render on black and then on white and subtract to derive the premultiplied alpha.

Native widgets like scrollbars don't show up when you render to memory, and it's unclear how to fix that.

    
    

    
  
If you complete disable native themes then you should get something usable rendered for all widgets, although it's possible that's broken since it's rarely used.

You should be able to use nsIPresShell::RenderDocument and a suitably tricked-out gfxImageSurface to render into an RGBA memory buffer and preserve alpha, although the cairo image surface byte order might not be what you want.

For the notifications, you can use a chrome event listener for the "MozAfterPaint" event, although that may not be easy or even possible for you. But you could patch in a C++ notification system at the same level as the nsPresContext NotifyPaintEvent stuff.

But if you find yourself wanting to send input or be window-system-independent then I'd urge you to look closer at Chris' headless work.

    
    

    
  
I'm the IMVU developer Chad mentioned.

Our work is perhaps not quite headless.  There is still a win32 window heirarchy that the web browser lives in, it's just that for our overlays, these are hidden or offscreen windows.  I haven't looked at the headless patch, but I suspect 'headless' might mean basically a new type of widget implementation?



Briefly, to get this to work:  We are hosting a web browser in an offscreen/hidden hwnd, and using nsPresShell::RenderDocument, with a gfxImageSurface.

(In fact, I added a new call to nsPresShell, which is one of the 2 patches Chad mentioned, but this was only because we are embedding xulrunner, and the gfx stuff is on the "internal linkage" side)

The other patch was a seam I inserted into nsViewManager::UpdateWidgetArea I think, to get dirty region updates.  First I tried calling SetViewObserver, and chaining my own class in between the 'real' nsIViewObserver, (which is always nsPresShell I think?), to intercept the Paint() call which passes the dirty region in a parameter.  This didn't work for the offscreen/hidden windows because the offscreen/hidden regions are being clipped out and never painted.  I suspect your suggestions on paint events would have this same behavior, because the regions were never painted.

UpdateWidgetArea looked to be the right place to catch the dirty regions before they were clipped out, so I inserted a hook there to call us back.  We store and merge dirty regions as these notifications come in, and when it's time to render (our texture in the 3d view) call nsPresShell::RenderDocument to get the updated pixels and copy them into out texture.

Really, the work was relatively painless.  It's very convenient that cairo is already working in premultiplied alpha.

Sending events to embedded browser was also pretty easy.  I have only hooked up mouse events so far, but we just create and populate the event struct, and call nsIViewManager::DispatchEvent

so, to recap: our 'headless' work was just creating a hidden window to host the web browser, inserting a seam to get dirty region notifications, calling RenderDocument to render into a 32-bit rgba image, and synthesizing events and calling DispatchEvent.


As for the win32 controls (scrollbars etc) not showing up, I have planned to try disabling the native themes as you suggest.  We are completely sidestepping the issue of Flash or other plugins.

    
    

    
  
@Dusty:
It looks like we've used the same methods, but the headless backend provides a more convenient access to the properties needed to do this, and there's also a convenient embedding interface (based on the gtk2 one) for use in C applications.

The UpdateWidgetArea patch sounds interesting - I've wanted to provide a mode to do something like this, but haven't gotten round to it yet. It would be useful to see the source.

Our current working is you provide a memory address with width/height/stride information and this will be used as a base for all rendering. Using GLib, we send 'update' signals when areas of this region have changed and are ready to upload (if you want) to a texture. One thing we do that might be advantageous to you is that we can contain multiple windows on a single surface (for things like pop-ups).

By default, we don't create the image surface with an alpha channel (so things are faster), but there's again a convenience function to turn this on - it hooks into nsIWidget::GetTransparencyMode too, so I guess other parts of Mozilla know what to do (I wrote a blog post on it: http://chrislord.net/blog/Software/neat-new-cluttermozembed-feature.enlighten?showcomments=yes)

We have two methods for plugins; Windowless plugins just work with no messing about by drawing to a temporary xpixmap surface and copying back to the image surface - this is slow, but it works. Windowed plugins use XEmbed to overlay onto the page and it's expected that an embedder will use XComposite for them to look correct (and input will just have to be disabled if the page has some kind of transformation applied). These are both obviously Linux-specific.

For the record, disabling native themes works fine.

@Roc:
Cool, thanks. In the meantime, I'll continue development and try to catch up with some of what you guys are doing for the out-of-process bits and the platform in general.

    
    

    
  
Sorry for the maybe dumb question, but I can't figure this out...  How do you disable the native theme?

    
    

    
  
In the GTK2 backend, in the Makefile.in there's a variable NATIVE_THEME_SUPPORT that you can just comment out, in which case the native backend won't be built. 

In the more general sense, just don't implement the native theme interface ("@mozilla.org/chrome/chrome-native-theme;1")

    
    

    
  


  
This is the most recent diff between mozilla-central and the headless backend - I've tried to address issues raised in previous comments. Behaviour isn't perfect, currently - Scrolling child windows doesn't work quite correctly (which you see with drop-downs) and there are a load of warnings on the terminal when closing a modal dialog (no idea about this one).

I'm including it now as those are both probably small easily-fixed issues and I'd like to get this merged, if possible, sooner rather than later, as keeping up with the development pace of mozilla-central is getting pretty difficult :)

        Attachment #410546 -
        Attachment is obsolete: true

        Attachment #415622 -
        Flags: review?(roc)

        Attachment #415622 -
        Flags: review?(roc) → review?(karlt)

    
    

    
  
Comment on attachment 415622 [details] [diff] [review]
Latest diff between mozilla-central and offscreen/headless

I started looking at this.  I still have plenty to look at but I might as well
post the comments I have so far.

Can you include function names and more context in future patches, please?
These can be added automatically as described here:
https://developer.mozilla.org/en/Mercurial_FAQ#Configuration

No need to post another patch yet.  I'll continue looking at this.

>+++ b/browser/components/shell/src/Makefile.in	Wed Dec 02 12:12:45 2009 +0000
>@@ -59,6 +59,13 @@
> ifeq ($(MOZ_WIDGET_TOOLKIT), gtk2)
> CPPSRCS = nsGNOMEShellService.cpp
> endif
>+ifeq ($(MOZ_WIDGET_TOOLKIT), headless)
>+CPPSRCS = nsGNOMEShellService.cpp
>+REQUIRES	+= \
>+		mozgnome \
>+		thebes \
>+		$(NULL)
>+endif

REQUIRES doesn't actually do anything.
Most were removed a few months ago, I think.

>+++ b/config/autoconf.mk.in	Wed Dec 02 12:12:45 2009 +0000
>@@ -519,6 +521,9 @@
> MOZ_QT_CFLAGS		= @MOZ_QT_CFLAGS@
> MOZ_QT_LIBS		= @MOZ_QT_LIBS@
> 
>+MOZ_CAIRO_HEADLESS_CFLAGS	= @MOZ_CAIRO_HEADLESS_CFLAGS@
>+MOZ_CAIRO_HEADLESS_LIBS		= @MOZ_CAIRO_HEADLESS_LIBS@

When I first read this, I (incorrectly) guessed these were flags for building
cairo for headless gecko.

I think it would be clearer if these were just MOZ_HEADLESS_*.
(Gecko doesn't ever not have cairo these days.)

>+BUILD_DATE=@BUILD_DATE@

MOZ_BUILD_DATE and obj/config/buildid are similar.
Could these be used instead of adding another variable?

>+++ b/dom/base/nsDOMClassInfo.cpp	Wed Dec 02 12:12:45 2009 +0000
>@@ -6219,14 +6219,14 @@
>   {
>     nsDependentJSString str(::JS_ValueToString(cx, id));
> 
>+    const char *cstring = NS_ConvertUTF16toUTF8(str).get();
>+
>     if (win->IsInnerWindow()) {
> #ifdef DEBUG_PRINT_INNER
>-      printf("Property '%s' resolve on inner window %p\n",
>-             NS_ConvertUTF16toUTF8(str).get(), (void *)win);
>-#endif
>-    } else {
>-      printf("Property '%s' resolve on outer window %p\n",
>-             NS_ConvertUTF16toUTF8(str).get(), (void *)win);
>+      printf("Property '%s' resolve on inner window %p\n", cstring, (void *)win);
>+#endif
>+    } else {
>+      printf("Property '%s' resolve on outer window %p\n", cstring, (void *)win);
>     }
>   }
> #endif

NS_ConvertUTF16toUTF8 is a class which holds the memory for the converted
string.  This temporary would have been destroyed by the tiime it is used
here.

But I don't this we want this change anyway.  It only saves code when
DEBUG_PRINT_INNER is set, and other times it costs a conversion.

>+++ b/gfx/thebes/public/gfxPangoFonts.h	Wed Dec 02 12:12:45 2009 +0000
>@@ -43,6 +43,7 @@
> #include "gfxTypes.h"
> #include "gfxFont.h"
> 
>+#include "nsVoidArray.h"

It looks like this should not be necessary.

>+++ b/gfx/thebes/src/Makefile.in	Wed Dec 02 12:12:45 2009 +0000
>@@ -129,6 +129,28 @@
> EXTRA_DSO_LDOPTS += $(ZLIB_LIBS) $(XLDFLAGS) $(XLIBS) $(CAIRO_FT_LIBS)
> endif
> 
>+ifeq ($(MOZ_WIDGET_TOOLKIT),headless)
>+CPPSRCS +=      gfxPlatformHeadless.cpp \
>+		gfxPangoFonts.cpp \

There is a little inconsistency here wrt thebes/public/Makefile.in, which
takes MOZ_PANGO into consideration.

> ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2)
> CXXFLAGS += $(MOZ_PANGO_CFLAGS)
>+DEFINES += -DMOZ_ENABLE_GTK2
>+endif

Would MOZ_WIDGET_GTK2 be suitable instead of adding MOZ_ENABLE_GTK2?

>+ifeq ($(MOZ_WIDGET_TOOLKIT),headless)
>+CXXFLAGS += $(MOZ_PANGO_CFLAGS)
>+DEFINES += -DMOZ_ENABLE_HEADLESS

And similarly MOZ_WIDGET_HEADLESS here?
Also in layout/generic/Makefile.in.

>+ifdef MOZ_X11
>+CFLAGS += $(MOZ_GTK2_CFLAGS)
>+CXXFLAGS += $(MOZ_GTK2_CFLAGS)

Should these be conditional on MOZ_ENABLE_GTK2_PLUGINS instead of MOZ_X11?

>+++ b/gfx/thebes/src/cairo-xlib-utils.c	Wed Dec 02 12:12:45 2009 +0000
>@@ -211,6 +211,9 @@
>     cairo_bool_t ret;
> 
>     target = cairo_get_group_target (cr);
>+    if (cairo_surface_get_type (target) != CAIRO_SURFACE_TYPE_XLIB)
>+      return False;
>+
>     cairo_surface_get_device_offset (target, &device_offset_x, &device_offset_y);
>     d = cairo_xlib_surface_get_drawable (target);
> 

Ah, yes, the surface type should be checked before calling
cairo_xlib_surface_get_drawable, but there is a comment later in the function
indicating why the type is checked late.

Would it make sense to move the type check and cairo_xlib_surface_get_drawable
to after the rect count check?  The !d check shouldn't be necessary with the
type check.

>-  
>+
>     if (result) {
>         result->surface = NULL;
>         result->uniform_alpha = False;
>         result->uniform_color = False;
>     }
>-    
>+

Please try to avoid unnecessary whitespace changes as they make checking blame
a bit more effort.  (There are quite a few in this file.)

>+++ b/gfx/thebes/src/cairo-xlib-utils.h	Wed Dec 02 12:12:45 2009 +0000
>@@ -41,6 +41,10 @@
> #include "cairo.h"
> #include <X11/Xlib.h>
> 
>+#if defined(MOZ_ENABLE_HEADLESS)
>+#include "nsRect.h"
>+#endif
>+

>+#if defined(MOZ_ENABLE_HEADLESS)
>+                           const nsIntRect dirtyRect,
>+#endif

No need for these changes.  cairo-xlib-utils.h is actually unused. 

>+++ b/layout/generic/nsObjectFrame.cpp	Wed Dec 02 12:12:45 2009 +0000
>@@ -2838,6 +2846,9 @@
>   *static_cast<Window*>(value) = GDK_WINDOW_XID(gdkWindow);
> #endif
>   return NS_OK;
>+#elif defined(MOZ_ENABLE_GTK2_PLUGINS)
>+  *static_cast<Window*>(value) = GDK_ROOT_WINDOW();
>+  return NS_OK;
> #else
>   return NS_ERROR_NOT_IMPLEMENTED;

I wouldn't have thought that providing the root window would be useful, and
indicating the lack of support with NS_ERROR_NOT_IMPLEMENTED might be a better
option.

If having the root window is useful, can you add a comment to explain why,
please?

>@@ -4868,8 +4879,13 @@
>   aContext->Translate(pluginRect.pos);
> 
>   Renderer renderer(window, mInstance, pluginSize, pluginDirtyRect);
>+#if defined(MOZ_ENABLE_GTK2_PLUGINS) && defined(MOZ_ENABLE_HEADLESS)
>+  renderer.Draw(aContext, window->width, window->height, pluginDirtyRect,
>+                rendererFlags, nsnull);
>+#else
>   renderer.Draw(aContext, window->width, window->height,
>                 rendererFlags, nsnull);
>+#endif

Should we be doing this optimization for other renderers also?

(Just writing this down because it took me a while to work it out:

 AFAIK, the only reason for not translating to the origin of the dirty rect
 and using the dirty rect width instead of the window width is that this is
 only an optional dirty rect and _get_rectangular_clip then wouldn't know
 whether or not a clip was necessary.  We could have always clipped to the
 dirty rect instead of the window rect when the NativeRenderer doesn't give us
 a clip, but that won't work with our Expose event dirty rect workaround for
 Flash Player.)

>   else {
>+#if defined(MOZ_ENABLE_GTK2_PLUGINS) && defined(MOZ_ENABLE_HEADLESS)
>+    /* We are now here because the numClipRects we passed is 0 */
>+    clipRect.x = 0;
>+    clipRect.y = 0;
>+#else
>     // NPRect members are unsigned, but
>     // we should have been given a clip if an offset is -ve.
>     NS_ASSERTION(offsetX >= 0 && offsetY >= 0,
>                  "Clip rectangle offsets are negative!");
>     clipRect.x = offsetX;
>     clipRect.y = offsetY;
>+#endif
>     clipRect.width  = mWindow->width;
>     clipRect.height = mWindow->height;
>   }

Is this change still necessary with suggested change to _draw_with_xlib_direct?
I don't follow how offsets could be ignored.

>+++ b/modules/plugin/base/src/nsPluginNativeWindowGtk2.cpp	Wed Dec 02 12:12:45 2009 +0000
>@@ -241,6 +241,10 @@
>   GdkWindow *parent_win = gdk_window_lookup((XID)window);
>   mSocketWidget = gtk_socket_new();
> 
>+  //To gracefully handle the socket being destroyed due to destruction
>+  //of its container widget we want to know if the socket still exists.
>+  g_object_add_weak_pointer(G_OBJECT (mSocketWidget), (gpointer *)&mSocketWidget);
>+

This should no longer be necessary now that we have

  g_signal_connect(mSocketWidget, "destroy",
                   G_CALLBACK(gtk_widget_destroyed), &mSocketWidget);

>+#ifdef MOZ_COMPOSITED_PLUGINS
>+  /* store away a reference to the socketwidget */
>+  mPlugWindow = (mSocketWidget);
>+#endif
>+

I don't think CreateXEmbedWindow is used when MOZ_COMPOSITED_PLUGINS is
defined.

    
    

    
  
This patch includes changes to a number of areas of code, and some of the
changes are not specific to headless widgets.  Usually it works best to
separate such changes into separate bugs as this makes
reviewing/merging/archaeology/etc easier, however having all the changes
together like this is really a consequence of using a separate repository for
a significant period of time.

I'll request review from the appropriate modules owners/peers.  Some may ask
the reasons for the changes or ask that a patch be posted to a separate bug.

One thing that we need to think about is how we land this on mozilla-central.
Is it best to pull from incubator/offscreen?  Does Mozilla have a policy on
which tags can be applied to m-c?  (i.e. Is there a review process for tags?)

    
    

    
  
Comment on attachment 415622 [details] [diff] [review]
Latest diff between mozilla-central and offscreen/headless

Requesting review from Kai for the changes in security/manager/ssl/src/

        Attachment #415622 -
        Flags: review?(kaie)

        Attachment #415622 -
        Flags: review?(bzbarsky)

    
    

    
  
Comment on attachment 415622 [details] [diff] [review]
Latest diff between mozilla-central and offscreen/headless

bz for the change to netwerk/cache/src/nsDiskCacheMap.cpp

    
    

    
  
Can someone explain what the purpose of that change is, and why it's needed?

    
    

    
  
Comment on attachment 415622 [details] [diff] [review]
Latest diff between mozilla-central and offscreen/headless

>+++ b/toolkit/components/filepicker/src/nsFilePicker.js.in	Wed Dec 02 12:12:45 2009 +0000
>@@ -240,14 +240,18 @@
>       dump("file picker couldn't get base window\n"+ex+"\n");
>     }
>     try {
>-      if (parentWin)
>-        parentWin.blurSuppression = true;
>+      try {
>+        if (parentWin)
>+          parentWin.blurSuppression = true;
>+      } catch (ex) { }
>       parent.openDialog("chrome://global/content/filepicker.xul",
>                         "",
>                         "chrome,modal,titlebar,resizable=yes,dependent=yes",
>                         o);
>-      if (parentWin)
>-        parentWin.blurSuppression = false;
>+      try {
>+        if (parentWin)
>+          parentWin.blurSuppression = false;
>+      } catch (ex) { }
> 
>       this.mFilterIndex = o.retvals.filterIndex;
>       this.mFilesEnumerator = o.retvals.files;

I don't see why nsXULWindow::SetBlurSuppression would fail.
Is a nsChromeTreeOwner without a XULWindow involved here?

But I'll request review from neil for this and the changes in 
toolkit/components/filepicker/
toolkit/components/viewconfig/ and
toolkit/content/

        Attachment #415622 -
        Flags: review?(neil)

    
    

    
  
Comment on attachment 415622 [details] [diff] [review]
Latest diff between mozilla-central and offscreen/headless

>+PACKAGE_FILE = mozheadless.pkg

PACKAGE_FILE has now been removed (bug 461888) and mozheadless.pkg won't be
needed.  Similarly with widget_headless.pkg in widget/src/headless.

>+++ b/gfx/thebes/src/gfxHeadlessNativeRenderer.cpp	Wed Dec 02 12:12:45 2009 +0000

>+    GdkDrawable *drawable = (GdkDrawable *)
>+        gfxPlatformHeadless::GetPlatform()->GetGdkDrawable(gfxSurface);

Apart from the dirtyRect, from which gfxGdkNativeRenderer would also benefit,
this use of gfxPlatformHeadless is the only difference I see between this file
and gfxGdkNativeRenderer.  This headless version is really a GDK renderer, so
I think it makes sense to just reuse gfxGdkNativeRenderer with the dirtyRect
changes and some conditional compilation to pick up the appropriate
gfxPlatform implementation.

e.g. http://hg.mozilla.org/mozilla-central/annotate/2f769a583cf2/gfx/thebes/src/gfxFT2Fonts.cpp#l37

There are also a few extra headers included.
Not sure which of those are actually necessary.

>+++ b/modules/plugin/sdk/samples/basic/unix/Makefile	Wed Dec 02 12:12:45 2009 +0000

>+CPPSRCS         = plugin.cpp
>+
>+SHARED_LIBRARY_LIBS = ../../common/$(LIB_PREFIX)plugingate_s.$(LIB_SUFFIX)

I don't know whether these changes are appropriate.  Usually these (and the
other variables added) go into a "Makefile.in".

It's probably best not to include these changes (including the line in
toolkit/toolkit-makefiles.sh) with the headless implementation here, and
instead submit a patch in a different bug report if desired.

>diff -r a5e1b195ecf7 -r 939897db9e57 toolkit/Makefile.in
>--- a/toolkit/Makefile.in	Tue Dec 01 15:06:31 2009 -0800
>+++ b/toolkit/Makefile.in	Wed Dec 02 12:12:45 2009 +0000
>@@ -57,6 +57,10 @@
> PARALLEL_DIRS += system/unixproxy
> endif
> 
>+ifneq (,$(filter headless,$(MOZ_WIDGET_TOOLKIT)))
>+DIRS += system/unixproxy
>+endif

Probably PARALLEL_DIRS here (or just add headless to
$(filter gtk2 qt,$(MOZ_WIDGET_TOOLKIT)) above.

>+++ b/toolkit/library/libxul-rules.mk	Wed Dec 02 12:12:45 2009 +0000

>+ifeq ($(MOZ_WIDGET_TOOLKIT),headless)
>+EXTRA_DSO_LDOPTS += $(MOZ_PANGO_LIBS)
>+endif
>+

With gtk widgets, this is now (since Sept 2008) done in
toolkit/library/Makefile.in, so this would be best moved to there.

    
    

    
  
Comment on attachment 415622 [details] [diff] [review]
Latest diff between mozilla-central and offscreen/headless

>+  try {
>+    window.setCursor("auto");
>+  } catch(ex) { }

>+      try {
>+        if (parentWin)
>+          parentWin.blurSuppression = true;
>+      } catch (ex) { }
So, why are these calls expected to throw?

>+try {
>+  const gClipboardHelper = Components.classes[nsClipboardHelper_CONTRACTID].getService(nsIClipboardHelper);
>+} catch (e) {
>+  // nsClipboardHelper may not be implemented
>+  // If it isn't, we need to continue
>+}
Please use nsClipboardHelper_CONTRACTID in Components.classes ? ... : null; instead to check for this. (But I was surprised that you might want to run about:config headless, yet not bother to support copy & paste.)

    
    

    
  
(In reply to comment #97)
>So, why are these calls expected to throw?
[Also getAttention(), which I forgot to paste in here.]

    
    

    
  
First, just a foreword to say that some small changes that really weren't intended have slipped into this patch - Given how long I've been maintaining this branch, it's been hard to separate actual changes and things that should have been distribution patches.

With that in mind, ...

(In reply to comment #90)
> (From update of attachment 415622 [details] [diff] [review])
> I started looking at this.  I still have plenty to look at but I might as well
> post the comments I have so far.
> 
> Can you include function names and more context in future patches, please?
> These can be added automatically as described here:
> https://developer.mozilla.org/en/Mercurial_FAQ#Configuration
> 
> No need to post another patch yet.  I'll continue looking at this.

Sure thing - of course, you can check this branch out too from http://hg.mozilla.org/incubator/offscreen (but I will make sure to generate more useful patches in future)

> >+++ b/browser/components/shell/src/Makefile.in	Wed Dec 02 12:12:45 2009 +0000
> >@@ -59,6 +59,13 @@
> > ifeq ($(MOZ_WIDGET_TOOLKIT), gtk2)
> > CPPSRCS = nsGNOMEShellService.cpp
> > endif
> >+ifeq ($(MOZ_WIDGET_TOOLKIT), headless)
> >+CPPSRCS = nsGNOMEShellService.cpp
> >+REQUIRES	+= \
> >+		mozgnome \
> >+		thebes \
> >+		$(NULL)
> >+endif
> 
> REQUIRES doesn't actually do anything.
> Most were removed a few months ago, I think.

Cool - This patch began over a year ago, so that wasn't the case back then :)

> >+++ b/config/autoconf.mk.in	Wed Dec 02 12:12:45 2009 +0000
> >@@ -519,6 +521,9 @@
> > MOZ_QT_CFLAGS		= @MOZ_QT_CFLAGS@
> > MOZ_QT_LIBS		= @MOZ_QT_LIBS@
> > 
> >+MOZ_CAIRO_HEADLESS_CFLAGS	= @MOZ_CAIRO_HEADLESS_CFLAGS@
> >+MOZ_CAIRO_HEADLESS_LIBS		= @MOZ_CAIRO_HEADLESS_LIBS@
> 
> When I first read this, I (incorrectly) guessed these were flags for building
> cairo for headless gecko.
> 
> I think it would be clearer if these were just MOZ_HEADLESS_*.
> (Gecko doesn't ever not have cairo these days.)

Sounds perfectly reasonable.

> >+BUILD_DATE=@BUILD_DATE@
> 
> MOZ_BUILD_DATE and obj/config/buildid are similar.
> Could these be used instead of adding another variable?

I wasn't aware of MOZ_BUILD_DATE, I guess this can be removed.

> >+++ b/dom/base/nsDOMClassInfo.cpp	Wed Dec 02 12:12:45 2009 +0000
> >@@ -6219,14 +6219,14 @@
> >   {
> >     nsDependentJSString str(::JS_ValueToString(cx, id));
> > 
> >+    const char *cstring = NS_ConvertUTF16toUTF8(str).get();
> >+
> >     if (win->IsInnerWindow()) {
> > #ifdef DEBUG_PRINT_INNER
> >-      printf("Property '%s' resolve on inner window %p\n",
> >-             NS_ConvertUTF16toUTF8(str).get(), (void *)win);
> >-#endif
> >-    } else {
> >-      printf("Property '%s' resolve on outer window %p\n",
> >-             NS_ConvertUTF16toUTF8(str).get(), (void *)win);
> >+      printf("Property '%s' resolve on inner window %p\n", cstring, (void *)win);
> >+#endif
> >+    } else {
> >+      printf("Property '%s' resolve on outer window %p\n", cstring, (void *)win);
> >     }
> >   }
> > #endif
> 
> NS_ConvertUTF16toUTF8 is a class which holds the memory for the converted
> string.  This temporary would have been destroyed by the tiime it is used
> here.
> 
> But I don't this we want this change anyway.  It only saves code when
> DEBUG_PRINT_INNER is set, and other times it costs a conversion.

Yes, I'm not sure what I was thinking here... Should be removed.

> >+++ b/gfx/thebes/public/gfxPangoFonts.h	Wed Dec 02 12:12:45 2009 +0000
> >@@ -43,6 +43,7 @@
> > #include "gfxTypes.h"
> > #include "gfxFont.h"
> > 
> >+#include "nsVoidArray.h"
> 
> It looks like this should not be necessary.

Ok.

> >+++ b/gfx/thebes/src/Makefile.in	Wed Dec 02 12:12:45 2009 +0000
> >@@ -129,6 +129,28 @@
> > EXTRA_DSO_LDOPTS += $(ZLIB_LIBS) $(XLDFLAGS) $(XLIBS) $(CAIRO_FT_LIBS)
> > endif
> > 
> >+ifeq ($(MOZ_WIDGET_TOOLKIT),headless)
> >+CPPSRCS +=      gfxPlatformHeadless.cpp \
> >+		gfxPangoFonts.cpp \
> 
> There is a little inconsistency here wrt thebes/public/Makefile.in, which
> takes MOZ_PANGO into consideration.
> 
> > ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2)
> > CXXFLAGS += $(MOZ_PANGO_CFLAGS)
> >+DEFINES += -DMOZ_ENABLE_GTK2
> >+endif
> 
> Would MOZ_WIDGET_GTK2 be suitable instead of adding MOZ_ENABLE_GTK2?

MOZ_ENABLE_GTK2 is used when a backend may want to use gtk2 features, regardless of the widget toolkit. MOZ_WIDGET_GTK2 would mark sections specifically for the gtk2 backend, ENABLE would mark shared sections between backends that may use gtk2 features (such as plugins).

> >+ifeq ($(MOZ_WIDGET_TOOLKIT),headless)
> >+CXXFLAGS += $(MOZ_PANGO_CFLAGS)
> >+DEFINES += -DMOZ_ENABLE_HEADLESS
> 
> And similarly MOZ_WIDGET_HEADLESS here?
> Also in layout/generic/Makefile.in.
> 
> >+ifdef MOZ_X11
> >+CFLAGS += $(MOZ_GTK2_CFLAGS)
> >+CXXFLAGS += $(MOZ_GTK2_CFLAGS)
> 
> Should these be conditional on MOZ_ENABLE_GTK2_PLUGINS instead of MOZ_X11?

Yes, they should.

> >+++ b/gfx/thebes/src/cairo-xlib-utils.c	Wed Dec 02 12:12:45 2009 +0000
> >@@ -211,6 +211,9 @@
> >     cairo_bool_t ret;
> > 
> >     target = cairo_get_group_target (cr);
> >+    if (cairo_surface_get_type (target) != CAIRO_SURFACE_TYPE_XLIB)
> >+      return False;
> >+
> >     cairo_surface_get_device_offset (target, &device_offset_x, &device_offset_y);
> >     d = cairo_xlib_surface_get_drawable (target);
> > 
> 
> Ah, yes, the surface type should be checked before calling
> cairo_xlib_surface_get_drawable, but there is a comment later in the function
> indicating why the type is checked late.
> 
> Would it make sense to move the type check and cairo_xlib_surface_get_drawable
> to after the rect count check?  The !d check shouldn't be necessary with the
> type check.

Yes, I think it would - it's probably a good idea to check d anyway, but I managed to miss that comment later on.

> >-  
> >+
> >     if (result) {
> >         result->surface = NULL;
> >         result->uniform_alpha = False;
> >         result->uniform_color = False;
> >     }
> >-    
> >+
> 
> Please try to avoid unnecessary whitespace changes as they make checking blame
> a bit more effort.  (There are quite a few in this file.)

Apologies for that, I did quite a bit of work on this file (that I eventually threw away).

> >+++ b/gfx/thebes/src/cairo-xlib-utils.h	Wed Dec 02 12:12:45 2009 +0000
> >@@ -41,6 +41,10 @@
> > #include "cairo.h"
> > #include <X11/Xlib.h>
> > 
> >+#if defined(MOZ_ENABLE_HEADLESS)
> >+#include "nsRect.h"
> >+#endif
> >+
> 
> >+#if defined(MOZ_ENABLE_HEADLESS)
> >+                           const nsIntRect dirtyRect,
> >+#endif
> 
> No need for these changes.  cairo-xlib-utils.h is actually unused. 

Right - again, legacy.

> >+++ b/layout/generic/nsObjectFrame.cpp	Wed Dec 02 12:12:45 2009 +0000
> >@@ -2838,6 +2846,9 @@
> >   *static_cast<Window*>(value) = GDK_WINDOW_XID(gdkWindow);
> > #endif
> >   return NS_OK;
> >+#elif defined(MOZ_ENABLE_GTK2_PLUGINS)
> >+  *static_cast<Window*>(value) = GDK_ROOT_WINDOW();
> >+  return NS_OK;
> > #else
> >   return NS_ERROR_NOT_IMPLEMENTED;
> 
> I wouldn't have thought that providing the root window would be useful, and
> indicating the lack of support with NS_ERROR_NOT_IMPLEMENTED might be a better
> option.
> 
> If having the root window is useful, can you add a comment to explain why,
> please?

I think the situation was that some plugins expect to get a window back from this and don't check for failure, so providing any window is better than none. But this is wrong, and it'd be better for there to be a way to set a window in the headless backend for use with plugins, or something like that.

Reading the comment above in that function though, do windowless plugins ever directly read native mouse events?

> >@@ -4868,8 +4879,13 @@
> >   aContext->Translate(pluginRect.pos);
> > 
> >   Renderer renderer(window, mInstance, pluginSize, pluginDirtyRect);
> >+#if defined(MOZ_ENABLE_GTK2_PLUGINS) && defined(MOZ_ENABLE_HEADLESS)
> >+  renderer.Draw(aContext, window->width, window->height, pluginDirtyRect,
> >+                rendererFlags, nsnull);
> >+#else
> >   renderer.Draw(aContext, window->width, window->height,
> >                 rendererFlags, nsnull);
> >+#endif
> 
> Should we be doing this optimization for other renderers also?
> 
> (Just writing this down because it took me a while to work it out:
> 
>  AFAIK, the only reason for not translating to the origin of the dirty rect
>  and using the dirty rect width instead of the window width is that this is
>  only an optional dirty rect and _get_rectangular_clip then wouldn't know
>  whether or not a clip was necessary.  We could have always clipped to the
>  dirty rect instead of the window rect when the NativeRenderer doesn't give us
>  a clip, but that won't work with our Expose event dirty rect workaround for
>  Flash Player.)

To be honest, I can't remember the reason we did this...

> >   else {
> >+#if defined(MOZ_ENABLE_GTK2_PLUGINS) && defined(MOZ_ENABLE_HEADLESS)
> >+    /* We are now here because the numClipRects we passed is 0 */
> >+    clipRect.x = 0;
> >+    clipRect.y = 0;
> >+#else
> >     // NPRect members are unsigned, but
> >     // we should have been given a clip if an offset is -ve.
> >     NS_ASSERTION(offsetX >= 0 && offsetY >= 0,
> >                  "Clip rectangle offsets are negative!");
> >     clipRect.x = offsetX;
> >     clipRect.y = offsetY;
> >+#endif
> >     clipRect.width  = mWindow->width;
> >     clipRect.height = mWindow->height;
> >   }
> 
> Is this change still necessary with suggested change to _draw_with_xlib_direct?
> I don't follow how offsets could be ignored.

Or this...

> >+++ b/modules/plugin/base/src/nsPluginNativeWindowGtk2.cpp	Wed Dec 02 12:12:45 2009 +0000
> >@@ -241,6 +241,10 @@
> >   GdkWindow *parent_win = gdk_window_lookup((XID)window);
> >   mSocketWidget = gtk_socket_new();
> > 
> >+  //To gracefully handle the socket being destroyed due to destruction
> >+  //of its container widget we want to know if the socket still exists.
> >+  g_object_add_weak_pointer(G_OBJECT (mSocketWidget), (gpointer *)&mSocketWidget);
> >+
> 
> This should no longer be necessary now that we have
> 
>   g_signal_connect(mSocketWidget, "destroy",
>                    G_CALLBACK(gtk_widget_destroyed), &mSocketWidget);

Ok.

> >+#ifdef MOZ_COMPOSITED_PLUGINS
> >+  /* store away a reference to the socketwidget */
> >+  mPlugWindow = (mSocketWidget);
> >+#endif
> >+
> 
> I don't think CreateXEmbedWindow is used when MOZ_COMPOSITED_PLUGINS is
> defined.

Given we don't define MOZ_COMPOSITED_PLUGINS and I've never messed with the hildon stuff, I've no idea how this got there - but I wasn't responsible for much of the plugins code, so it's probably another piece of legacy. Looks like it can be removed.


I'll try to make the necessary changes when I can, but I only have my spare time to work on this now, so I probably won't be able to get round to it for a few days.

    
    

    
  
(In reply to comment #94)
> Can someone explain what the purpose of that change is, and why it's needed?

We were seeing crashes without this change I think - If you run without a profile directory set, I guess there are things that may fail in this file and it doesn't cover every failure case?

I wasn't the one to introduce this change, so I'm not sure - I'll see if I can find out the reason for this, but if I remember correctly, it was to stop a crash when there's no profile directory set, or to stop a warning, something along those lines.

    
    

    
  
With respect to the try/catch blocks in the javascript files, these were added for the situation where a backend doesn't implement certain things (like blur, for example) - It was really a stop-gap solution so that things like file dialogs, etc. would work while the backend wasn't complete.

        Attachment #415622 -
        Flags: review?(bzbarsky) → review?(cbiesinger)

    
    

    
  
Comment on attachment 415622 [details] [diff] [review]
Latest diff between mozilla-central and offscreen/headless

I think Christian would be a better reviewer here.  I'm not quite sure how we can even end up with an nsDiskCacheRecord if we have no mRecordArray, unless it's the code in nsDiskCacheDevice::BindEntry that creates random stack records... in which case maybe the right fix is there, not here.

    
    

    
  
(In reply to comment #99)
Thanks, Chris.

> > > ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2)
> > > CXXFLAGS += $(MOZ_PANGO_CFLAGS)
> > >+DEFINES += -DMOZ_ENABLE_GTK2
> > >+endif
> > 
> > Would MOZ_WIDGET_GTK2 be suitable instead of adding MOZ_ENABLE_GTK2?
> 
> MOZ_ENABLE_GTK2 is used when a backend may want to use gtk2 features,
> regardless of the widget toolkit. MOZ_WIDGET_GTK2 would mark sections
> specifically for the gtk2 backend, ENABLE would mark shared sections between
> backends that may use gtk2 features (such as plugins).

The logic of this statement makes sense, but this isn't actually needed at
this stage (or any longer).  MOZ_ENABLE_GTK2 is only set when MOZ_WIDGET_GTK2
is set and only used to include "gfxPlatformGtk.h".  MOZ_ENABLE_GTK2_PLUGINS
covers the remaining cases.

> > >+++ b/layout/generic/nsObjectFrame.cpp	Wed Dec 02 12:12:45 2009 +0000
> > >@@ -2838,6 +2846,9 @@
> > >   *static_cast<Window*>(value) = GDK_WINDOW_XID(gdkWindow);
> > > #endif
> > >   return NS_OK;
> > >+#elif defined(MOZ_ENABLE_GTK2_PLUGINS)
> > >+  *static_cast<Window*>(value) = GDK_ROOT_WINDOW();
> > >+  return NS_OK;
> > > #else
> > >   return NS_ERROR_NOT_IMPLEMENTED;
> > 
> > I wouldn't have thought that providing the root window would be useful, and
> > indicating the lack of support with NS_ERROR_NOT_IMPLEMENTED might be a better
> > option.
> > 
> > If having the root window is useful, can you add a comment to explain why,
> > please?
> 
> I think the situation was that some plugins expect to get a window back from
> this and don't check for failure, so providing any window is better than none.
> But this is wrong, and it'd be better for there to be a way to set a window in
> the headless backend for use with plugins, or something like that.

I'll leave it up to you what you think is best for now.

> Reading the comment above in that function though, do windowless plugins ever
> directly read native mouse events?

I'm not clear which comment you are referring to here.
Most of the comments in this function are XP_WIN specific.

But to try to answer your question, nsPluginInstanceOwner::ProcessEvent()
doesn't directly send native mouse events so plugins wouldn't get their
events.  But sometimes windowless plugins create their own toplevel windows,
which get events.

    
    

    
  
(In reply to comment #101)
> With respect to the try/catch blocks in the javascript files, these were added
> for the situation where a backend doesn't implement certain things (like blur,
> for example) - It was really a stop-gap solution so that things like file
> dialogs, etc. would work while the backend wasn't complete.
Well without more information I would have thought that those functions such as GetAttention should just be stubbed out to return NS_OK; // nothing to do here.

    
    

    
  
Comment on attachment 415622 [details] [diff] [review]
Latest diff between mozilla-central and offscreen/headless

I've only really paged through the headless-specific code here.  That's all I
intend to do for a review.

The main question is whether "headless" is an appropriate name for the
widget/embedding implementations.  Is the idea that the client uses
GLib/GObject to interface with a headless Gecko?  If so, this is in one sense
a GLib implementation.

Or is GLib just used in the implementation, not intended the client, and could
be removed at some time in the future, without changing external interfaces?

If GLib is part of the client interface, Chris, what do you think about
calling this glib-headless?  Does that make sense?  No need to rename classes.
It would just be renaming widget and embedding directories, and possibly some
configure.in variables.

An observation was that nsClipboard is conditional on MOZ_ENABLE_GTK_PLUGINS
in the Makefile and MOZ_X11 in nsWidgetFactory.cpp.  I understand
MOZ_ENABLE_GTK_PLUGINS is because of the gtk dependency even though
nsClipboard is not really related to plugins.  Maybe this is a case for
MOZ_ENABLE_GTK, but this issue doesn't need to hold up landing this.

As a general comment, it would be best/easiest to get something landed on
mozilla-central before merging gets even more difficult.  Dropping the
unnecessary changes in platform-generic files for now would probably speed
this up.  That's mostly a comment about the security, netwerk, and toolkit changes.
I've commented on all the things in other code that should be sorted out.

    
    

    
  
Just a note, I've taken everything in comment #90 into account so far (and will do so for the rest of the comments) and am fixing up this back-end at the moment.

In reply to comment #105, I'm fine with renaming it glib-headless instead of cairo-headless - it makes more sense. It was always intended that GLib would be removed, but the embedding API relies heavily on it for the moment, as do small parts in the widget back-end, so this isn't feasible for now.

If I did rename it glib-headless, what directories would need to be renamed? Would it be possible to just stick with the 'headless' directory names for now?

    
    

    
  
(In reply to comment #106)
Thank you, Chris.

The directories that I was thinking of were embedding/browser/(glib-)headless and widget/src/(glib-)headless.

I'm actually not so worried about Makefile/preprocessor variable names at this stage, but was keen to get the directory names right ASAP.  If it seemed likely that the glib dependency would be removed then I'd be OK with keeping the "headless" names, but it sounds like it is not certain that this is going to happen.  ("hg rename" could do much of this, though I expect some Makefile changes would be necessary.)

    
    

    
  


  
I noticed we'd never attached our patches to Gecko 1.9.1 required for our 3D overlay rendering.  Here they are in case they're interesting.

We don't plan to upgrade to 1.9.2 or 2.0 in the short term due to focus issues, but I hope Gecko 2.0 provides a more direct API.

    
    

    
  
Have you filed bugs on the focus issues you have?

    
    

    
  
Check to see whether the focus issues are bug 533245.

    
    

    
  
smaug: Just filed bug Bug 611425, thanks.

karlt: They look related, thanks.  I don't know whether they're dups or not.

We can move focus-related discussion there.

    
  
Whiteboard: [imvu]

    
    

    
  
(In reply to comment #110)
> We don't plan to upgrade to 1.9.2 or 2.0 in the short term due to focus issues,
> but I hope Gecko 2.0 provides a more direct API.

In Gecko 2.0 you should be able to adapt this headless approach to do accelerated rendering direct to a GL/D3D9/D3D10 render target, by overriding nsIWidget::GetLayerManager and possibly extending LayerManagerOGL/D3D9/D3D10.

    
    

    
  
Thanks roc - I'm looking forward to those days.  Do you expect any issues, performance or otherwise, with including a 3D scene viewer updating at 60 Hz in the layer tree?  We render in software for 1/3 of our users, because D3D is unreliable and slower than software on many common Intel GPUs.  The Intel GPU is probably fine for 2D layer composition, but we'd want to render the 3D scene in software, and copy the framebuffer to a gecko layer...

Today, we have a plug-in that lets us position arbitrary HWNDs in the document tree.  Simultaneously, we have code that renders gecko web browsers into a texture and efficiently positions them relative to objects in the 3D scene.  Architecturally, there could be big wins if we could depend on Gecko for all of that.

    
    

    
  
(In reply to comment #115)
> Thanks roc - I'm looking forward to those days.  Do you expect any issues,
> performance or otherwise, with including a 3D scene viewer updating at 60 Hz in
> the layer tree?  We render in software for 1/3 of our users, because D3D is
> unreliable and slower than software on many common Intel GPUs.  The Intel GPU
> is probably fine for 2D layer composition, but we'd want to render the 3D scene
> in software, and copy the framebuffer to a gecko layer...

That's certainly doable; WebGL does exactly that. I can't speak to the performance of software rendering but if it works at all, it can be fitted into the layer system.

> Today, we have a plug-in that lets us position arbitrary HWNDs in the document
> tree.  Simultaneously, we have code that renders gecko web browsers into a
> texture and efficiently positions them relative to objects in the 3D scene. 
> Architecturally, there could be big wins if we could depend on Gecko for all of
> that.

Interesting!

I haven't looked at your app, but I wonder if it could be written as a XUL extension or even a Web app, using CSS 3D transforms (which we'll do immediately post-FF4) and -moz-element...

    
    

    
  
Comment on attachment 415622 [details] [diff] [review]
Latest diff between mozilla-central and offscreen/headless

Cleaning up my review queue... this review seems outdated at this point.

        Attachment #415622 -
        Flags: review?(cbiesinger)

    
    

    
  
Oleg has been doing some great work on this, assigning to him (feel free to unassign if this is a bit presumptuous!)
Assignee: chrislord.net → romaxa

        Attachment #415622 -
        Flags: review?(neil)

        Attachment #415622 -
        Flags: review?(karlt)

        Attachment #415622 -
        Flags: review?(kaie)
Whiteboard: [imvu] → [imvu] [sudweb]
Blocks: slimerjs
Alias: headless

    
    

    
  
(In reply to Hallvord R. M. Steen [:hallvors] from comment #119)
> Completing this would be good for SlimerJS -
> https://github.com/laurentj/slimerjs/issues/80

I've not heard anything about it for a long time, but have you investigated IPCLite? https://wiki.mozilla.org/Embedding/IPCLiteAPI

    
  
Whiteboard: [imvu] [sudweb] → [imvu] [sudweb][tpi:-]
See Also:  → 1338004

    
    

    
  
For those still interested in headless browsing I've started work on a different approach to headless browsing over in bug 1338004. This bug is old enough and the new approach is different enough that I thought it warranted a new bug to not bring irrelevant history along.

    
    

    
  
The bug assignee didn't login in Bugzilla in the last 7 months.

:spohl, could you have a look please?

For more information, please visit auto_nag documentation.


Assignee: romaxa → nobody
Flags: needinfo?(spohl.mozilla.bugs)

    
    

    
  
Headless mode was implemented in bug 1338004 (and dependent bugs). Even though the approach was different, the result of having headless mode is the same, so closing as duplicate seems appropriate here.


Status: NEW → RESOLVED
Closed: 2 years ago
Flags: needinfo?(spohl.mozilla.bugs)
Resolution: --- → DUPLICATE

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: