Closed Bug 582616 Opened 9 years ago Closed 9 years ago

Sharing back-end for Android

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set

Tracking

(fennec2.0b2+)

VERIFIED FIXED
Tracking Status
fennec 2.0b2+ ---

People

(Reporter: mbrubeck, Assigned: blassey)

References

()

Details

Attachments

(1 file, 2 obsolete files)

Allow the new Sharing feature in Fennec 2.0 to discover, list, and launch third-party Android applications.
OS: All → Android
Hardware: All → ARM
Hardware: ARM → All
tracking-fennec: ? → 2.0b1+
Assignee: mbrubeck → blassey.bugs
Attached patch WIP patch (obsolete) — Splinter Review
This works, bug I'm putting this up to get feedback on the js api.  For now I've added getHandlerAppsForSharingMIMEType() to nsIDOMWindowUtils.

The example use is pretty simple:
let handlers = winUtils.getHandlerAppsForSharingMIMEType("image/png");

I tested it by enumerating the sharing handlers for PNGs on about:home as buttons and launching them.
Attachment #468591 - Flags: feedback?(mark.finkle)
getHandlerAppsForSharingMIMEType seems like a good API. I initially thought of returning a nsIHandlerInfo (or nsIMIMEInfo) and retrieving the list of nsIHandlerApp from that.

We could implement an interface like this for Maemo too, but it might need a little hacking.

getHandlerAppsForSharing(...) might be enough of a name, but I think we need to find a better home for the method. Any ideas for that?

nsIExternalHelperAppService seems like a good place, but the IDL file seems weak. nsIExternalProtocolService seems like it's a good interface, but it's all protocol based.

Maybe nsIMIMEService?
Comment on attachment 468591 [details] [diff] [review]
WIP patch

fb+ for the API direction
Attachment #468591 - Flags: feedback?(mark.finkle) → feedback+
Attached patch patch (obsolete) — Splinter Review
nsIExternalHelperAppService's idl is kinda weak, but I think its the most appropriate place to hang this function.
Attachment #468591 - Attachment is obsolete: true
Attachment #468623 - Flags: review?(mwu)
Attachment #468623 - Flags: review?(dwitte)
Comment on attachment 468623 [details] [diff] [review]
patch

Let's change to the spec outlined here:
http://etherpad.mozilla.com:9000/sharing-interface
Attachment #468623 - Flags: review?(mwu)
Attachment #468623 - Flags: review?(dwitte)
tracking-fennec: 2.0b1+ → 2.0b2+
Depends on: 598268
Attached patch patchSplinter Review
Attachment #468623 - Attachment is obsolete: true
Attachment #477413 - Flags: review?(mwu)
Attachment #477413 - Flags: review?(mark.finkle)
Comment on attachment 477413 [details] [diff] [review]
patch

The non-Android parts look OK to me.
Attachment #477413 - Flags: review?(mark.finkle) → review+
Duplicate of this bug: 598268
Comment on attachment 477413 [details] [diff] [review]
patch

Hm, will these reviews be enough? We're touching docshell code and adding interfaces.. might need a sr?

>diff --git a/docshell/build/nsDocShellModule.cpp b/docshell/build/nsDocShellModule.cpp
>--- a/docshell/build/nsDocShellModule.cpp
>+++ b/docshell/build/nsDocShellModule.cpp
>@@ -110,6 +113,9 @@ NS_GENERIC_FACTORY_CONSTRUCTOR(PlatformL
> #ifdef MOZ_ENABLE_DBUS
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsDBusHandlerApp)
> #endif 
>+#if defined(ANDROID)
>+NS_GENERIC_FACTORY_CONSTRUCTOR(nsExternalSharingAppService)
>+#endif
#ifdef for consistency with the rest of the file.

> 
> // session history
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsSHEntry)
>@@ -134,6 +140,9 @@ NS_DEFINE_NAMED_CID(NS_LOCALHANDLERAPP_C
> #ifdef MOZ_ENABLE_DBUS
> NS_DEFINE_NAMED_CID(NS_DBUSHANDLERAPP_CID);
> #endif
>+#if defined(ANDROID)
>+NS_DEFINE_NAMED_CID(NS_EXTERNALSHARINGAPPSERVICE_CID);
>+#endif
#ifdef

> NS_DEFINE_NAMED_CID(NS_SHENTRY_CID);
> NS_DEFINE_NAMED_CID(NS_HISTORYENTRY_CID);
> NS_DEFINE_NAMED_CID(NS_SHTRANSACTION_CID);
>@@ -158,6 +167,9 @@ const mozilla::Module::CIDEntry kDocShel
> #ifdef MOZ_ENABLE_DBUS
>   { &kNS_DBUSHANDLERAPP_CID, false, NULL, nsDBusHandlerAppConstructor },
> #endif
>+#if defined(ANDROID)
>+  { &kNS_EXTERNALSHARINGAPPSERVICE_CID, false, NULL, nsExternalSharingAppServiceConstructor },
>+#endif
#ifdef

>   { &kNS_SHENTRY_CID, false, NULL, nsSHEntryConstructor },
>   { &kNS_HISTORYENTRY_CID, false, NULL, nsSHEntryConstructor },
>   { &kNS_SHTRANSACTION_CID, false, NULL, nsSHTransactionConstructor },
>@@ -199,6 +211,9 @@ const mozilla::Module::ContractIDEntry k
> #ifdef MOZ_ENABLE_DBUS
>   { NS_DBUSHANDLERAPP_CONTRACTID, &kNS_DBUSHANDLERAPP_CID },
> #endif
>+#if defined(ANDROID)
>+  { NS_EXTERNALSHARINGAPPSERVICE_CONTRACTID, &kNS_EXTERNALSHARINGAPPSERVICE_CID },
>+#endif
#ifdef

>diff --git a/uriloader/exthandler/android/nsExternalSharingAppService.cpp b/uriloader/exthandler/android/nsExternalSharingAppService.cpp
>new file mode 100644
>--- /dev/null
>+++ b/uriloader/exthandler/android/nsExternalSharingAppService.cpp
>diff --git a/uriloader/exthandler/android/nsExternalSharingAppService.h b/uriloader/exthandler/android/nsExternalSharingAppService.h
>new file mode 100644

namespace mozilla for this file so we can shorten some things.

>--- /dev/null
>+++ b/uriloader/exthandler/android/nsExternalSharingAppService.h
>+  /* additional members */
>+};
>+
>+#endif
#endif /* NS_EXTERNAL_SHARING_APP_SERVICE_H */
Attachment #477413 - Flags: review?(mwu) → review+
Comment on attachment 477413 [details] [diff] [review]
patch

>+NS_IMETHODIMP
>+nsExternalSharingAppService::GetSharingApps(const nsAString & aMIMEType,
>+                                            PRUint32 *aLen NS_OUTPARAM,
>+                                            nsISharingHandlerApp ***aHandlers NS_OUTPARAM)
>+{
>+  nsresult rv;
>+  NS_NAMED_LITERAL_STRING(sendAction, "android.intent.action.SEND");
>+  nsCOMPtr<nsIMutableArray> array = do_CreateInstance(NS_ARRAY_CONTRACTID, &rv);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+  NS_ConvertUTF16toUTF8 nMimeType(aMIMEType);
>+  if (mozilla::AndroidBridge::Bridge()) {
if (!AndroidBridge::Bridge())
    return NS_OK;

>+    mozilla::AndroidBridge::Bridge()->GetHandlersForMimeType(nMimeType.get(),
>+                                                             array, nsnull, 
>+                                                             sendAction);
>+    array->GetLength(aLen);
>+    *aHandlers = 
>+      static_cast<nsISharingHandlerApp**>(NS_Alloc(sizeof(nsISharingHandlerApp*)
>+                                                   * *aLen));
>+    for (int i = 0; i < *aLen; i++) {
>+      rv = array->QueryElementAt(i, nsISharingHandlerApp::GetIID(), 
>+                                 (void**)(*aHandlers + i));
>+      NS_ENSURE_SUCCESS(rv, rv);
>+    }
>+    return NS_OK;
extra return.

>+  }
>+  return NS_OK;
>+}
(In reply to comment #9)
> Comment on attachment 477413 [details] [diff] [review]
> patch
> 
> Hm, will these reviews be enough? We're touching docshell code and adding
> interfaces.. might need a sr?
> 
> >diff --git a/docshell/build/nsDocShellModule.cpp b/docshell/build/nsDocShellModule.cpp
> >--- a/docshell/build/nsDocShellModule.cpp
> >+++ b/docshell/build/nsDocShellModule.cpp
> >@@ -110,6 +113,9 @@ NS_GENERIC_FACTORY_CONSTRUCTOR(PlatformL
> > #ifdef MOZ_ENABLE_DBUS
> > NS_GENERIC_FACTORY_CONSTRUCTOR(nsDBusHandlerApp)
> > #endif 
> >+#if defined(ANDROID)
> >+NS_GENERIC_FACTORY_CONSTRUCTOR(nsExternalSharingAppService)
> >+#endif
> #ifdef for consistency with the rest of the file.

using #if defined(ANDROID) because maemo will slot right in here when bug 582621 lands
Comment on attachment 477413 [details] [diff] [review]
patch

I don't think sr is strictly necessary, but it can't hurt either
Attachment #477413 - Flags: superreview?(pavlov)
Attachment #477413 - Flags: superreview?(pavlov) → superreview+
pushed http://hg.mozilla.org/mozilla-central/rev/8a6554580c4e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Flags: in-litmus?(tchung)
Verified fix on Mozilla/5.0 (Android; Linux armv71; rv:2.0b7pre) Gecko/20101008 Firefox/4.0b7pre Fennec/2.0b2pre
Status: RESOLVED → VERIFIED
Fwiw, the classid/contractid should have gone in nsDocShellCID.h, not in the IDL....
You need to log in before you can comment on or make changes to this bug.