285080 - We should put the frozen embedding contracts into nsEmbedCID.h

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Contracts which are "frozen" (that is, for which we either guarantee or require
support of certain interfaces) should be moved into the SDK in nsEmbedCID.h.

Also, I have a proposal to use contracts that include the string
"embeddor.implemented" or some such (see
http://lxr.mozilla.org/seamonkey/source/xpfe/components/build/nsModule.cpp#181)
for things that we expect embeddors to implement, in the future.  That will
differentiate those contracts from the ones that our embedding layer itself
implements.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Actually, ignore the additional proposal thing...  I should put that in a
separate bug.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch for prompt service

I didn't touch embeddors for now (because they sometimes use <> for headers and
sometimes use "" and I can't tell which they would want here).	I also didn't
touch any places that don't use REQUIRES, since I wasn't sure they would
build...

Comment 3

[Darin Fisher]

Comment on attachment 176544 [details] [diff] [review]
Patch for prompt service

>Index: embedding/base/nsEmbedCID.h

>+/**
>+ * Prompt Service ContractID
>+ *   The prompt service is the way to pose various prompts, alerts,
>+ *   and confirmation dialogs to the user.
>+ * 
>+ * This contract implements the following interfaces:
>+ * nsIPromptService
>+ */
>+#define NS_PROMPTSERVICE_CONTRACTID \
>+ "@mozilla.org/embedcomp/prompt-service;1"

This comment is good, but maybe you should add something about the fact
that embedders may override this ContractID with their own implementation.

r=darin

Comment 4

[Benjamin Smedberg]

Hey, cc me on these ;)

For windowwatcher, I think we should move the contractid to nsEmbedCID.h and add
a %{C++ #include "nsEmbedCID.h" %} block to the file, so that existing code
doesn't break. The CIDs should be hidden and inaccessible.

Comment 5

[Boris Zbarsky [:bzbarsky]]

bsmedberg, sorry.  I meant to cc you and missed, somehow.

What about the random IID define that windowwatcher has?  It's the same value as
the IID defined by the idl, but a different constant name.  :(

Added the following to the comment for prompt-service:

 * Embedders may override this ContractID with their own implementation if they
 * want more control over the way prompts, alerts, and confirmation dialogs are
 * presented to the user.

And checked it in.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

the comments should make clear whether to use createInstance or getService for
the contractid... I think the web browser one could be improved in this respect
(the prompt service one is ok I guess, since it has a "service" in the name...)

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Like so?

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 177617 [details] [diff] [review]
Like so?

great, thanks

Comment 9

[Darin Fisher]

Comment on attachment 177617 [details] [diff] [review]
Like so?

BTW, the method is named createInstanceByContractID ;-)

In nsNetCID.h, I use the phrase: "service implementing nsIFoo" and "component
implementing nsIBar"

that's not so great either (kinda vague actually), but it avoids talking about
ContractID-taking functions named createInstance() and getService() that don't
exactly exist.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 177617 [details] [diff] [review]
Like so?

Checked in, with the function names fixed.

Comment 11

[Darin Fisher]

*** Bug 270240 has been marked as a duplicate of this bug. ***

Comment 12

[Boris Zbarsky [:bzbarsky]]

Fixed enough.  People can file more bugs as desired.