Status

Core Graveyard
Embedding: GRE Core
RESOLVED FIXED
13 years ago
2 years ago

People

(Reporter: Benjamin Smedberg, Assigned: Benjamin Smedberg)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

13 years ago
This is a spinoff for part two of bug 302099 - embedding libxul with a
standardized XRE_InitEmbedding function. I apparently have this working on
windows now, by golly.
(Assignee)

Comment 1

13 years ago
Created attachment 195107 [details] [diff] [review]
Build winembed on libxul, rev. 1

This build winembed on libxul. I think that I'd like to just drop support for
winembed/mfcembed/gtkmozembed based on seamonkey as I go along, rather than
wrapping everything in ifdefs.

This code also demonstrates that we could really use some glue to help deal
with the functions exported from libxul, not just libxpcom. I'm trying to think
of a decent API.
Attachment #195107 - Flags: review?(darin)
(Assignee)

Comment 2

13 years ago
Wow, I must be out of it... I already did XRE_InitEmbedding, this is the testing
and verification process :-(
Summary: Embedding libxul, part 2 - XRE_InitEmbedding → Make winembed use libxul

Comment 3

13 years ago
Comment on attachment 195107 [details] [diff] [review]
Build winembed on libxul, rev. 1

>Index: profile/dirserviceprovider/standalone/Makefile.in

>+DEFINES += -DXPCOM_GLUE

hmm... ok, so we've decided that embedders must always link against
the full glue, and this library is only useful to embedders, so it
too must assume that the application will be linking against the
full glue, right?

btw, i think it would be nice if embedders did not need to know about
all the directory service keys that need to be defined in order to
have a useful profile.	it would be nice if XRE_InitEmbedding took
care of supplying a these keys and only required that the embedder
specify the location of the profile directory.	for example, if a
future version of the runtime uses sqlite based storage, we wouldn't
want embedders to have to re-link their code with a new directory
service provider that knew how to specify the location of the sqlite
database file(s).


>Index: embedding/tests/mfcembed/Makefile.in

>+	xulapp \

so, in theory you should be able to build an embedding app against
a runtime that does not support XUL (--disable-xul), but perhaps
that is not an interesting target anymore.  thoughts?  (i realize
that "xulapp" doesn't imply use of XUL.)


>Index: embedding/tests/mfcembed/MfcEmbed.cpp

> #ifdef XPCOM_GLUE
>+    const char *xpcomPath = GRE_GetXPCOMPath();
>+    if (!xpcomPath || NS_FAILED(XPCOMGlueStartup(xpcomPath))) {
>+        MessageBox(NULL, "Could not initialize XPCOM. Perhaps the GRE\n"
>+                   "is not installed or could not be found?", "MFCEmbed", MB_OK | MB_ICONERROR);
>         return FALSE;
>     }
> #endif

Can MfcEmbed really be compiled without XPCOM_GLUE?  maybe remove
this #ifdef ?


>Index: embedding/tests/winEmbed/WebBrowserChrome.cpp

>+// NON-FROZEN APIS!
>+#include "nsIWebNavigation.h"
>+#if 0
>+#include "nsIDocShellTreeItem.h"
>+#endif

Dump the "#if 0" code?	nsIWebNavigation is practically frozen :-)


>+#if 0 // non-frozen APIs, biesi says this code is probably unneeded
>     nsCOMPtr<nsIDocShellTreeItem> dsti = do_QueryInterface(mWebBrowser);
>     dsti->SetItemType(nsIDocShellTreeItem::typeContentWrapper);
>+#endif

If this is not needed, then remove it?	nsDocShell's default constructor
sets mItemType to typeContent, so yeah... this is probably not needed.


>+        char intstr[10];
>+        _snprintf(intstr, sizeof(intstr) - 1, "%i", info1);
>+        intstr[sizeof(intstr) - 1] = '\0';
>+        status.Append(intstr);

This code is begging to be factored into a subroutine.	A static
helper function in this source module at least! ;-)


>Index: embedding/tests/winEmbed/winEmbed.cpp

>+    char xulPath[_MAX_PATH];
>+    strcpy(xulPath, xpcomPath);
>+    strcat(xulPath, "\\xul.dll");

danger.. buffer overflow potential.


>+    HINSTANCE xulModule = LoadLibraryEx(xulPath, NULL, 0);
>+    if (!xulModule)
>+        return 4;

"magic numbers are bad" ... but ok, this is just test code.


>+    // Scope all the XPCOM stuff
>+    {

maybe a subroutine would be better?


r=darin
Attachment #195107 - Flags: review?(darin) → review+
(Assignee)

Comment 4

13 years ago
> btw, i think it would be nice if embedders did not need to know about
> all the directory service keys that need to be defined in order to

I agree completely, but let's leave that for a second step. There are issues I
haven't reconciled in my head about profile startup.

> so, in theory you should be able to build an embedding app against
> a runtime that does not support XUL (--disable-xul), but perhaps
> that is not an interesting target anymore.  thoughts?  (i realize
> that "xulapp" doesn't imply use of XUL.)

Even minimo enables XUL. The Testing Matrix rules all, I'm going to ignore and
eventually remove as many of these configure flags as I can.

> Can MfcEmbed really be compiled without XPCOM_GLUE?  maybe remove
> this #ifdef ?

Oops, mfcembed changes weren't supposed to appear in this patch. I'll have
another patch for mfcembed later.

Fixed with nits on trunk. This won't actually work until attachment 195106 [details] [diff] [review] is
checked in.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.