Being able to create a standalone JAR package for Gecko and GeckoView will allow other Android developers to use GeckoView in their applications.

Bug 873569 is for moving Gecko assets (like libs, and omnijar) into the assets/ folder of the APK.

Temporary script for copying built JARs, SOs, omni.ja, and resources to library project directory.

Temporary patch to add option to GeckoView to open a URL in a new thread

Changes resources to non-constant IDs which is necessary for library projects.

WIP patch to adapt GeckoView to run without having control over an Activity

updated patches for allowing a page to load :)

Change resources to non-static variables which is necessary for a library project.

Comment on attachment 787232 [details] [diff] [review] add-new-tab-option.diff I know this was patch was rejected in another bug, but it is necessary to get a page to load in GeckoView. Until a proper GeckoView interface is defined, this will at least allow a page to be loaded.

Comment on attachment 787232 [details] [diff] [review] add-new-tab-option.diff Review of attachment 787232 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/GeckoView.java @@ +76,4 @@ > } > > + public void loadUrl(String uri, boolean openInNewTab) { > + Tabs.getInstance().loadUrl(uri, (openInNewTab ? Tabs.LOADURL_NEW_TAB : Tabs.LOADURL_NONE)); Boolean parameters are often a short-sighted design and obscure the caller code. I would much prefer this new method be named loadUrlInNewTab() or take a LOADURL flags parameter. Taking a flags parameter might be a better design because it emphasizes that this method is simply a pass-through to Tab.getInstance().

Changed to "loadUrlInNewTab" because changing to expose the flags variable means having to expose the Tabs flags to the 3rd party app using the library. While this is temporary anyway, it seems better to hide this info for now until the GeckoView interface is hashed out.

Comment on attachment 789798 [details] [diff] [review] add-new-tab-option.diff Review of attachment 789798 [details] [diff] [review]: ----------------------------------------------------------------- LGTM!

Comment on attachment 789771 [details] [diff] [review] constant-id.diff Review of attachment 789771 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, but I think you should simplify these long chains of `if/else` statements to just a sequence of `if` statements than each call `return`. You can then add an empty line between each `if` block to make these long methods easier to read.

Comment on attachment 789898 [details] [diff] [review] constant-id.diff Review of attachment 789898 [details] [diff] [review]: ----------------------------------------------------------------- LGTM! ::: mobile/android/base/BrowserApp.java @@ +1728,2 @@ > > + int itemId = item.getItemId(); Please make this itemId `final` like you did in the other methods.

add-new-tab-option.diff: https://hg.mozilla.org/integration/mozilla-inbound/rev/d17e4a0cba94 constant-id.diff: https://hg.mozilla.org/integration/mozilla-inbound/rev/a3cc1c802366

Comment on attachment 789898 [details] [diff] [review] constant-id.diff Holy cow. This patch will be very unpopular. What's the issue here?

A library project cannot have its resource IDs in R.java be static which means they can't be used as case labels. Otherwise, all the resources will be null.

(In reply to Mark Finkle (:mfinkle) from comment #18) > Comment on attachment 789898 [details] [diff] [review] > constant-id.diff > > Holy cow. This patch will be very unpopular. > > What's the issue here? Reading between the lines, stully is discovering that we strongly enforce having only one set of Android resource IDs and therefore only one R.java. You probably recall fighting with Sync to get the resources to work (or perhaps rnewman did that all): at that time, there was less support for having multiple resource sets that got merged together. It is a longer term project to make Fennec build better with multiple resource sets, like what stully is hacking in here. This shouldn't land as it is but is useful experimentation.

> A library project cannot have its resource IDs in R.java be static which means they can't be used as case labels. Otherwise, all the resources will be null. Correction, they cannot be final; static is okay.

> It is a longer term project to make Fennec build better with multiple resource sets, like what stully is hacking in here. This shouldn't land as it is but is useful experimentation. The problem isn't multiple resource sets. By default, resource IDs are static final ints. The value of a static final int will be inlined into the bytecode. This means that the memory location this int points to is going to be invalid when the classes are used in a library project. The solution to this is to make the resource IDs non-final at the cost of not being able to use them as case labels. If you want to keep them as final, every single resource reference in the code will be to be gotten by "getResources().getIdentifier(...)" which is much worse than just making the resource IDs non-final. Given the relatively small number of places in our code that resources are used as case labels, it seemed this was the lesser of two evils. The only other option is to have two versions of each file that uses resources IDs as case lebels, one with a case structure and one with an if-else structure. This idea, obviously, shouldn't even be entertained.

More info: http://tools.android.com/tips/non-constant-fields Based on this, I think we'll need to move away from switch statements and use ugly if/else blocks. BTW, I was able to get Fennec building with multiple resource sets in bug 901803 (the first obsoleted Cast SDK patch).

(In reply to Nick Alexander :nalexander from comment #20) > It is a longer term project to make Fennec build better with multiple > resource sets, like what stully is hacking in here. This shouldn't land as > it is but is useful experimentation. oops. I already landed the non-final resource ID patch yesterday. Should I back it out?

(In reply to Chris Peterson (:cpeterson) from comment #25) > (In reply to Nick Alexander :nalexander from comment #20) > > It is a longer term project to make Fennec build better with multiple > > resource sets, like what stully is hacking in here. This shouldn't land as > > it is but is useful experimentation. > > oops. I already landed the non-final resource ID patch yesterday. Should I > back it out? Nah, let it ride. I'm not thrilled about having Makefile.in changes land without build peer review and I'm with blassey on not liking the if/elif chains, but no sense churning for churns sake.

Everything needed to create zip files ready to be dropped into Eclipse for using GeckoView in a third party app.

Comment on attachment 790961 [details] [diff] [review] add-library-project-directory.diff Review of attachment 790961 [details] [diff] [review]: ----------------------------------------------------------------- This is moving in a good direction, but since there's no *build* per se (except moving some files -- keep that in the new Makefile.in), I'd like to see all of the zip/libs/assets logic moved into packager.mk. There'll be fewer gnarly ../.. chains. Also, prefer listing explicit files to update the zip with rather than globbing. Lists are good! Make sure to think about armv7/armv6/x86 too. ::: mobile/android/geckoview_library/.classpath @@ +4,5 @@ > + <classpathentry kind="src" path="gen"/> > + <classpathentry kind="con" path="com.android.ide.eclipse.adt.ANDROID_FRAMEWORK"/> > + <classpathentry exported="true" kind="con" path="com.android.ide.eclipse.adt.LIBRARIES"> > + <attributes> > + <attribute name="org.eclipse.jdt.launching.CLASSPATH_ATTR_LIBRARY_PATH_ENTRY" value="geckoview_library/libs/armeabi-v7a"/> x86? ARMv6? And how do the Fennec libraries (some of which are native) fit in here? I'm not thrilled with shipping a .classpath, but I'm not clear on how you intend this to be used. ::: mobile/android/geckoview_library/Makefile.in @@ +10,5 @@ > +include $(DEPTH)/config/autoconf.mk > + > +INSTALL_TARGETS += GECKOVIEW_LIBRARY > +GECKOVIEW_LIBRARY_DEST = $(CURDIR) > +GECKOVIEW_LIBRARY_FILES := AndroidManifest.xml \ nit: := \ \tfile1 \ \tfile2 \ @@ +25,5 @@ > + # Make empty directories to fit an Android project structure > + mkdir -p bin gen libs/armeabi-v7a src > + > + # JARs > + mv ../base/jars/* libs/ This will bust the previously built parts of the objdir. You can $(CP), but I think you might want to do this whole business in packager.mk. @@ +28,5 @@ > + # JARs > + mv ../base/jars/* libs/ > + > + # Mozglue (this should be in a directory named 'libs', not 'lib') > + cp ../../../dist/bin/libmozglue.so libs/armeabi-v7a/ Again, x86/armv6. We have fu to deal with this in packager.mk; you might need to extract a better framework. @@ +40,5 @@ > + cd ..; \ > + zip -r ../../dist/geckoview_library/geckoview_library.zip geckoview_library > + > + # The makefiles shouldn't be included in the zip > + zip -d ../../../dist/geckoview_library/geckoview_library.zip geckoview_library/backend.mk geckoview_library/Makefile What is going into this? Is it just a few static files (listed above) and then some of the jars and libs built above? If so, we should be doing this all in packager.mk. @@ +43,5 @@ > + # The makefiles shouldn't be included in the zip > + zip -d ../../../dist/geckoview_library/geckoview_library.zip geckoview_library/backend.mk geckoview_library/Makefile > + > + # Move the JARs and resources back > + mv libs/*.jar ../base/jars/ Well, that's good -- but prefer staging + $(CP). And $(ZIP), $(MKDIR), etc while you're at it. ::: toolkit/mozapps/installer/packager.mk @@ +344,5 @@ > INNER_ROBOCOP_PACKAGE=echo 'Testing is disabled - No Robocop for you' > endif > > +INNER_MAKE_GECKOVIEW_LIBRARY= \ > + cd fennec && \ nit: subshell for the scope of the cd. @@ +345,5 @@ > endif > > +INNER_MAKE_GECKOVIEW_LIBRARY= \ > + cd fennec && \ > + $(ZIP) -r ../geckoview_library/geckoview_assets.zip assets && \ Not clear why we have a separate assets zip. I'd prefer one zip with all the things. @@ +346,5 @@ > > +INNER_MAKE_GECKOVIEW_LIBRARY= \ > + cd fennec && \ > + $(ZIP) -r ../geckoview_library/geckoview_assets.zip assets && \ > + cd - Not clear on what cd - does. subshell is better.

Comment on attachment 792994 [details] [diff] [review] disable-content-providers.diff Review of attachment 792994 [details] [diff] [review]: ----------------------------------------------------------------- Maybe the content providers should be off by default and manually enabled by GeckoApp instead of adding hacks to GeckoView. ::: mobile/android/base/db/BrowserDB.java @@ +19,5 @@ > import java.util.List; > > public class BrowserDB { > public static String ABOUT_PAGES_URL_FILTER = "about:%"; > + private static boolean sDisableContentProviders; I think a name like `sAreContentProvidersDisabled` (or, flipping the logic, `sAreContentProvidersEnabled`) might be clearer because it describes a state, not an action. @@ +213,4 @@ > } > > public static boolean isReadingListItem(ContentResolver cr, String uri) { > + return (sDisableContentProviders ? false : sDb.isReadingListItem(cr, uri)); You can simplify these expressions to something like: return (!sDisableContentProviders && sDb.isReadingListItem(cr, uri));

Comment on attachment 792993 [details] [diff] [review] library-project.diff Review of attachment 792993 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/GeckoView.java @@ +129,5 @@ > + } > + > + public Activity getActivity() { > + return (Activity)mContext; > + }; remove extra semicolon. @@ +172,5 @@ > + public void disableCameraView() {} > + > + public void addAppStateListener(GeckoAppShell.AppStateListener listener) {} > + > + public void removeAppStateListener(GeckoAppShell.AppStateListener listener) {} Where are all these nop methods called from?

Updated manifest with permissions for GeckoView

Comment on attachment 793128 [details] [diff] [review] add-library-project-directory.diff Review of attachment 793128 [details] [diff] [review]: ----------------------------------------------------------------- I'm confused about how this will be consumed. Specifying .classpath suggests an Eclipse project (but then there should be .project, no)? The inclusion of Makefile and backend.mk is almost certainly not wanted. This is a strong f+ -- providing consumer build context might make this an r+. ::: mobile/android/geckoview_library/.classpath @@ +4,5 @@ > + <classpathentry kind="src" path="gen"/> > + <classpathentry kind="con" path="com.android.ide.eclipse.adt.ANDROID_FRAMEWORK"/> > + <classpathentry exported="true" kind="con" path="com.android.ide.eclipse.adt.LIBRARIES"> > + <attributes> > + <attribute name="org.eclipse.jdt.launching.CLASSPATH_ATTR_LIBRARY_PATH_ENTRY" value="geckoview_library/libs/armeabi-v7a"/> What was the word on all the different ABIs we support? ::: mobile/android/geckoview_library/Makefile.in @@ +11,5 @@ > + > +INSTALL_TARGETS += GECKOVIEW_LIBRARY > +GECKOVIEW_LIBRARY_DEST = $(CURDIR) > +GECKOVIEW_LIBRARY_FILES := \ > + AndroidManifest.xml \ nit: sort these. And two spaces at start rather than \t (Make is crazy). ::: toolkit/mozapps/installer/packager.mk @@ +343,5 @@ > else > INNER_ROBOCOP_PACKAGE=echo 'Testing is disabled - No Robocop for you' > endif > > +INNER_MAKE_GECKOVIEW_LIBRARY= \ Can we get a one-line comment explaining what this does right before the command? For example: # Create geckoview_library/geckoview_{assets,library}.zip for third-party GeckoView consumers. @@ +354,5 @@ > + cp bin/libmozglue.so bin/lib/libplugin-container.so ../mobile/android/geckoview_library/libs/$(ABI_DIR)/ && \ > + cp -R ../mobile/android/base/res ../mobile/android/geckoview_library && \ > + ( cd ../mobile/android && \ > + $(ZIP) -r ../../dist/geckoview_library/geckoview_library.zip geckoview_library) && \ > + $(ZIP) -d geckoview_library/geckoview_library.zip geckoview_library/backend.mk geckoview_library/Makefile I'm a bit confused here. You almost certainly don't want the processed Makefile or backend.mk included here -- that's going to include a ton of things that third parties won't want. (For example, includes like rules.mk.)

> I'm confused about how this will be consumed. Specifying .classpath suggests an Eclipse project (but then there should be .project, no)? Yes, it will be an Eclipse project. The .project file should have been included in the patch, but I guess I missed it. The updated patch has it included.

Comment on attachment 793127 [details] [diff] [review] library-project.diff Review of attachment 793127 [details] [diff] [review]: ----------------------------------------------------------------- Brad should probably review these GeckoView changes.

Comment on attachment 793125 [details] [diff] [review] disable-content-providers.diff Review of attachment 793125 [details] [diff] [review]: ----------------------------------------------------------------- Brad should probably review these GeckoView changes.

Comment on attachment 793197 [details] [diff] [review] add-library-project-directory.diff Review of attachment 793197 [details] [diff] [review]: ----------------------------------------------------------------- Looks good.

Comment on attachment 793125 [details] [diff] [review] disable-content-providers.diff Review of attachment 793125 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/GeckoView.java @@ +42,5 @@ > + // If running outside of a GeckoActivity (eg, from a library project), > + // load the native code and disable content providers > + if (!(context instanceof GeckoActivity)) { > + BrowserDB.setEnableContentProviders(false); > + } Just wanted to point out that this code won't get hit in Fennec because it is after the doInit check. This would also be true of any user that set doinit to fale, but they'd already be a little screwed anyway.

Comment on attachment 793127 [details] [diff] [review] library-project.diff Review of attachment 793127 [details] [diff] [review]: ----------------------------------------------------------------- You have the basics here (stubbed out or default implementations of GeckoInterface functions), but the simple cast of the Context to an Activity seems wrong. ::: mobile/android/base/GeckoView.java @@ +135,5 @@ > + return null; > + } > + > + public Activity getActivity() { > + return (Activity)mContext; not a fan of this. There is a reason that there is a separate getContext() and getActivity(). Also, if this were "the right thing to do", callers of getActivity() could instead call (Activity)getContext(). Instead, let's provide a BaseGeckoInterface class that takes an Activity in the constructor and stubs out or provides default implementations for everything else (like you've done here). To make things "just work" we can do an instanceof test on the context we get in the GeckoView constructor and construct a BaseGeckoInterface using that.

> callers of getActivity() could instead call (Activity)getContext(). AFAIK, views do not have a getActivity() function so the only way to get an activity is to cast the context given in the constructor or returned by getContext() to an activity. Regardless, other methods in the new BaseGeckoInterface need a context, so its constructor takes a context that is then cast to an activity. GeckoView checks that the context given to BaseGeckoInterface is, in fact, an instance of Activity.

Comment on attachment 793197 [details] [diff] [review] add-library-project-directory.diff Mike, could you look at the changes to packager.mk? Context: this series of patches wraps up some of the Android assets and some new project files for consumption by third party developers who want to embed GeckoView on Android. We create two zip files in the packaging step; third parties would configure an Eclipse (or other) project using the assets and the project files. 1. It just occurred to me that some of the build artifacts that get zipped in the packaging step might not be present during some of the |make package| invocations on build bots. If you see anything obvious, can you say? 2. Eventually we want to distribute these zips via the buildbots. Is there anything else needed except an UPLOAD_EXTRA_FILES line (like robocop.apk)? 3. Should we be building these zips only conditionally (say for local and Nightly builds only)? Thanks!

Comment on attachment 793197 [details] [diff] [review] add-library-project-directory.diff Review of attachment 793197 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/mozapps/installer/packager.mk @@ +355,5 @@ > + cp bin/libmozglue.so bin/lib/libplugin-container.so ../mobile/android/geckoview_library/libs/$(ABI_DIR)/ && \ > + cp -R ../mobile/android/base/res ../mobile/android/geckoview_library && \ > + ( cd ../mobile/android && \ > + $(ZIP) -r ../../dist/geckoview_library/geckoview_library.zip geckoview_library) && \ > + $(ZIP) -d geckoview_library/geckoview_library.zip geckoview_library/backend.mk geckoview_library/Makefile I'd rather see this as a $(MAKE) -C $(DEPTH)/mobile/android/geckoview_library package with a package rule in mobile/android/geckoview_library/Makefile.in That'd be more readable. Note $(ZIP) has a -x option to exclude files when creating the archive instead of removing them with -d afterwards.

Comment on attachment 793741 [details] [diff] [review] library-project.diff Review of attachment 793741 [details] [diff] [review]: ----------------------------------------------------------------- r=blassey with nits addressed ::: mobile/android/base/BaseGeckoInterface.java @@ +53,5 @@ > + } > + > + public SensorEventListener getSensorEventListener() { > + return null; > + } file some follow up bugs to add set<>Listener() methods @@ +55,5 @@ > + public SensorEventListener getSensorEventListener() { > + return null; > + } > + > + public void doRestart() {} file a follow up bug to implement this (and put the bug number in a comment) @@ +75,5 @@ > + } > + > + public void addPluginView(final View view, final RectF rect, final boolean isFullScreen) {} > + > + public void removePluginView(final View view, final boolean isFullScreen) {} guess what? file follow ups for implementations. Actually, file a follow up for each method that is just stubbed out to get an implementation ::: mobile/android/base/GeckoView.java @@ +45,5 @@ > > // If running outside of a GeckoActivity (eg, from a library project), > // load the native code and disable content providers > if (!(context instanceof GeckoActivity)) { > + setGeckoInterface(context); do your instanceof check here and create a BaseGeckoInterface to pass to setGeckoInterface(). Also, do a (getGeckoInterface() != null) check to make sure we're not overwriting the interface that the embedder has already set. @@ +97,5 @@ > requestRender(); > } > } > > + public static void setGeckoInterface(final Context context) { keep setGeckoInterface(GeckoInterface) because I'd like an implementer to be able to call it

(In reply to Shane Tully (:stully: from comment #45) > Created attachment 793741 [details] [diff] [review] > library-project.diff > > > callers of getActivity() could instead call (Activity)getContext(). > > AFAIK, views do not have a getActivity() function so the only way to get an > activity is to cast the context given in the constructor or returned by > getContext() to an activity. I was referring to GeckoInterface.getActivity()

Comment on attachment 793741 [details] [diff] [review] library-project.diff Review of attachment 793741 [details] [diff] [review]: ----------------------------------------------------------------- r=blassey with nits addressed ::: mobile/android/base/BaseGeckoInterface.java @@ +53,5 @@ > + } > + > + public SensorEventListener getSensorEventListener() { > + return null; > + } file some follow up bugs to add set<>Listener() methods @@ +55,5 @@ > + public SensorEventListener getSensorEventListener() { > + return null; > + } > + > + public void doRestart() {} file a follow up bug to implement this (and put the bug number in a comment) @@ +75,5 @@ > + } > + > + public void addPluginView(final View view, final RectF rect, final boolean isFullScreen) {} > + > + public void removePluginView(final View view, final boolean isFullScreen) {} guess what? file follow ups for implementations. Actually, file a follow up for each method that is just stubbed out to get an implementation ::: mobile/android/base/GeckoView.java @@ +45,5 @@ > > // If running outside of a GeckoActivity (eg, from a library project), > // load the native code and disable content providers > if (!(context instanceof GeckoActivity)) { > + setGeckoInterface(context); do your instanceof check here and create a BaseGeckoInterface to pass to setGeckoInterface(). Also, do a (getGeckoInterface() != null) check to make sure we're not overwriting the interface that the embedder has already set. @@ +97,5 @@ > requestRender(); > } > } > > + public static void setGeckoInterface(final Context context) { keep setGeckoInterface(GeckoInterface) because I'd like an implementer to be able to call it

Documentation: https://wiki.mozilla.org/Mobile/GeckoView

Comment on attachment 794744 [details] [diff] [review] add-library-project-directory.diff Review of attachment 794744 [details] [diff] [review]: ----------------------------------------------------------------- Almost there. You'll need someone else's review for the xml files, though. ::: mobile/android/geckoview_library/Makefile.in @@ +21,5 @@ > +include $(topsrcdir)/config/rules.mk > + > +package: > + # Make directory for the zips > + $(MKDIR) -p ../../../dist/geckoview_library Please use $(DIST) instead of ../../../dist. @@ +25,5 @@ > + $(MKDIR) -p ../../../dist/geckoview_library > + > + # Zip the assets > + cd ../../../dist/fennec; \ > + $(ZIP) -r ../geckoview_library/geckoview_assets.zip assets This is going to break some of the .so that need to *not* be deflated. See SZIP_LIBRARIES in packager.mk. @@ +29,5 @@ > + $(ZIP) -r ../geckoview_library/geckoview_assets.zip assets > + > + # Make empty directories to fit an Android project structure > + $(MKDIR) -p bin gen libs/$(ABI_DIR) src > + Please remove the various empty tabs in this file. @@ +41,5 @@ > + cp -R ../base/res . > + > + # Zip the directory > + cd ..; \ > + $(ZIP) -r ../../dist/geckoview_library/geckoview_library.zip geckoview_library --exclude geckoview_library/backend.mk geckoview_library/Makefile Does the zip need to have a root geckoview_library directory?

Comment on attachment 794925 [details] [diff] [review] add-library-project-directory.diff Review of attachment 794925 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/geckoview_library/Makefile.in @@ +25,5 @@ > + $(MKDIR) -p $(DIST)/geckoview_library > + > + # Zip the assets > + cd $(DIST)/fennec; \ > + $(ZIP) -0 -r ../geckoview_library/geckoview_assets.zip assets You're effectively making all assets uncompressed. BTW, since this is not going to be an apk, how is an apk going to be created from this? Because that's actually where that matters most, that some files are kept uncompressed... ::: toolkit/mozapps/installer/packager.mk @@ +345,5 @@ > endif > > +# Create geckoview_library/geckoview_{assets,library}.zip for third-party GeckoView consumers. > +INNER_MAKE_GECKOVIEW_LIBRARY= \ > + $(MAKE) -C ../mobile/android/geckoview_library package ABI_DIR=$(ABI_DIR) dist=$(DIST) You don't need to pass DIST.

Comment on attachment 794925 [details] [diff] [review] add-library-project-directory.diff Review of attachment 794925 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/geckoview_library/Makefile.in @@ +25,5 @@ > + $(MKDIR) -p $(DIST)/geckoview_library > + > + # Zip the assets > + cd $(DIST)/fennec; \ > + $(ZIP) -0 -r ../geckoview_library/geckoview_assets.zip assets As per the discussion on irc, you have my r+ if you remove that -0 and remove the dist=$(DIST) from packager.mk

Comment on attachment 794786 [details] [diff] [review] library-project.diff gah, wrong one

Comment on attachment 794942 [details] [diff] [review] add-library-project-directory.diff Carrying forward r=blassey from comment 50

Comment on attachment 794786 [details] [diff] [review] library-project.diff Carrying forward r=glandium from comment 58

https://hg.mozilla.org/integration/mozilla-inbound/rev/39ee92c06d6b https://hg.mozilla.org/integration/mozilla-inbound/rev/59f669c61da5 https://hg.mozilla.org/integration/mozilla-inbound/rev/fa20cf0458b5

I tried a build and package. After, <objdir>/dist/geckoview_library/*.zip files were created. When I tired to create an Eclipse project (what a nightmare) I ended up with some missing references to a "geckoview_library/bin/geckoview.jar". Any idea what's going on? There is no "bin/geckoview.jar" file in my ZIP files.

https://hg.mozilla.org/mozilla-central/rev/fa20cf0458b5 https://hg.mozilla.org/mozilla-central/rev/59f669c61da5 https://hg.mozilla.org/mozilla-central/rev/39ee92c06d6b

This seems to have broken l10n builds (repacks): https://tbpl.mozilla.org/?showall=1&jobname=l10n%20nightly&rev=416075f77249

(In reply to Mark Finkle (:mfinkle) from comment #63) > I tried a build and package. After, <objdir>/dist/geckoview_library/*.zip > files were created. > > When I tired to create an Eclipse project (what a nightmare) I ended up with > some missing references to a "geckoview_library/bin/geckoview.jar". Any idea > what's going on? There is no "bin/geckoview.jar" file in my ZIP files. Did you followed the instructions in https://wiki.mozilla.org/Mobile/GeckoView ? According to those instructions you have to build 2 eclipse projects: a library project (named GeckoView) which must be imported from unzipped geckoview_library.zip and another eclipse android project (e.g. GeckoViewExample made by stully) which uses the first library project. If you do all things correctly, after building GeckoViewExample project you will also build GeckoView library project. The "bin/geckoview.jar" is located under GeckoView/bin/geckoview.jar

(In reply to Mark Finkle (:mfinkle) from comment #63) > I tried a build and package. After, <objdir>/dist/geckoview_library/*.zip > files were created. > > When I tired to create an Eclipse project (what a nightmare) I ended up with > some missing references to a "geckoview_library/bin/geckoview.jar". Any idea > what's going on? There is no "bin/geckoview.jar" file in my ZIP files. Notice that although in GeckoViewExample project which stully uploaded in https://wiki.mozilla.org/Mobile/GeckoView the library project is called "geckoview_library" but after importing geckoview_library from the zipped file geckoview_library.zip the imported project in eclipse will have the name "GeckoView". I think this is the source of your problem. The solution is: Right_click on GeckoViewExample project in eclipse -> properties -> Android -> delete reference to geckoview_library (it must have a red cross beside itself) -> read "GeckoView" instead

I also have a problem too: I can import, compile and build both GeckoView library project and GeckoViewExample project in eclipse. But when I run the GeckoViewExample app on my Android 4.2.2 device, it issues the error: Unfortunately, GeckoView Example has stopped. -> OK I can't figure out what can be the problem. Anyone has any idea?

Backed out 39ee92c06d6b because GeckoView breaks Android's l10n repacks: https://hg.mozilla.org/integration/mozilla-inbound/rev/35d97694d436

This patch fixes the bug where any drawable used in a GeckoView-based Application would potentially reorder the R.java identifiers and cause lookups in org.mozilla.gecko compiled JARs to fail. The two resources where this happens is: shadow and scrollbar, since they are the only ones hit during typical use of GeckoView. The problem seems to be that the JARs in geckoview_library are compiled with specific values for the R.type.resource and when the Application that uses geckoview_library mixes in it's resources the R.java values can be changed. This is expected. What's not expected is that geckoview_library does not care about the changed values. It only knows about the original values. Long story short, we can make it work if we dynamically load the resource identifiers for any *core* GeckoView resource. 99% of the resources in the geckoview_library will never be needed by the Application, so we could remove them in a separate bug.

(In reply to Chris Peterson (:cpeterson) from comment #69) > Backed out 39ee92c06d6b because GeckoView breaks Android's l10n repacks: > https://hg.mozilla.org/integration/mozilla-inbound/rev/35d97694d436 Link to the bustage: https://tbpl.mozilla.org/?showall=1&jobname=l10n%20nightly&rev=416075f77249

This patch adds a mozconfig option (--disable-geckoview-library) so we can skip any GeckoView work during the packaging.

Disable geckoview-library on any l10n builds. We are not ready to make it work there.

Comment on attachment 797650 [details] [diff] [review] allow-mozconfig-disable Review of attachment 797650 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +5133,5 @@ > + MOZ_DISABLE_GECKOVIEW=1, > + MOZ_DISABLE_GECKOVIEW=) > + > +if test -n "$MOZ_DISABLE_GECKOVIEW"; then > + AC_DEFINE(MOZ_DISABLE_GECKOVIEW) You're not using the AC_DEFINE, so no need for it. (AC_SUBST for things accessed in makefiles, AC_DEFINE for things accessed in code (and even for that, AC_DEFINE is most of the time overkill)) That being said, since this is a l10n releng build specific option, it would be better not to add one to the already long list of configure flags available. Just keep the AC_SUBST, and set MOZ_DISABLE_GECKOVIEW in the l10n mozconfigs.

This patch removes the AC_DEFINE support and just keeps the AC_SUBST support

This patch disables GeckoView for l10n builds by exporting the MOZ_DISABLE_GECKOVIEW flag directly. Tested locally and it does disable the library from being packaged.

Comment on attachment 799445 [details] [diff] [review] disable-geckoview-l10n v0.2 Review of attachment 799445 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/config/mozconfigs/android-armv6/l10n-nightly @@ +33,5 @@ > > export JAVA_HOME=/tools/jdk6 > export MOZILLA_OFFICIAL=1 > export MOZ_PKG_SPECIAL=armv6 > +export MOZ_DISABLE_GECKOVIEW=1 I think you don't need to export, but okay either way.

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ff14d43346d https://hg.mozilla.org/integration/mozilla-inbound/rev/965457d9bc1e https://hg.mozilla.org/integration/mozilla-inbound/rev/9f3ef3de5617

https://hg.mozilla.org/mozilla-central/rev/9ff14d43346d https://hg.mozilla.org/mozilla-central/rev/965457d9bc1e https://hg.mozilla.org/mozilla-central/rev/9f3ef3de5617 https://hg.mozilla.org/mozilla-central/rev/cfcf5b7edc53

Backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/feffc2cf422e https://hg.mozilla.org/integration/mozilla-inbound/rev/0fa38163e3bf because https://hg.mozilla.org/mozilla-central/rev/9ff14d43346d didn't seem to have build peer review.

Comment on attachment 794942 [details] [diff] [review] add-library-project-directory.diff Actually, Mike already reviewed this, so I'm carrying it forward: https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=880118&attachment=794925

... and relanded: remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/cb3a85808d77 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/b7396ea8f548 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/2a9bde2ff860 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/fff320870b20

https://hg.mozilla.org/mozilla-central/rev/cb3a85808d77 https://hg.mozilla.org/mozilla-central/rev/b7396ea8f548 https://hg.mozilla.org/mozilla-central/rev/2a9bde2ff860 https://hg.mozilla.org/mozilla-central/rev/fff320870b20

I am using geckoview lib. I got an resource not found exception. I solved this excepiton in a tricky way. I unpack gecko-browser.jar, rm the R classes and then re-pack to gecko-browser.jar replace origin gecko-broser.jar. Then ant debug. install apk to device. This will result java.lang.NoClassDefFoundError: org.mozilla.gecko.R$styleable. Then copy the geckoview fold in gen/org/mozilla/,rename to gecko. Edit the gen/org/mozilla/gecko/R.java, change package name to org.mozilla.gecko then ant debug, install apk to device. It works. Because the classloader can load the right R.class which have the right int id value. And I found your are working on fix the issue.https://bugzilla.mozilla.org/attachment.cgi?id=797273&action=diff I think there are two methons to avoid R.java reorder. 1. getResources().getIdentifier(), as in the diff 2. Change the R.java's package name to "org.mozilla.geckoview", Do not package R.class into gecko-browser.jar. Actually I only try in a HelloProject, Because I don't known how to change the build script this project. Any suggestion is appreciated P.S. I tried another method. Change the gecko library project's package name to "org.mozilla.gecko". But I got a so runtime exception. Is gecko library project's package name must be "org.mozilla.geckoview"? (In reply to Mark Finkle (:mfinkle) from comment #70) > Created attachment 797273 [details] [diff] [review] > Pull resources dynamically to avoid R.java reorder > > This patch fixes the bug where any drawable used in a GeckoView-based > Application would potentially reorder the R.java identifiers and cause > lookups in org.mozilla.gecko compiled JARs to fail. The two resources where > this happens is: shadow and scrollbar, since they are the only ones hit > during typical use of GeckoView. > > The problem seems to be that the JARs in geckoview_library are compiled with > specific values for the R.type.resource and when the Application that uses > geckoview_library mixes in it's resources the R.java values can be changed. > This is expected. What's not expected is that geckoview_library does not > care about the changed values. It only knows about the original values. > > Long story short, we can make it work if we dynamically load the resource > identifiers for any *core* GeckoView resource. > > 99% of the resources in the geckoview_library will never be needed by the > Application, so we could remove them in a separate bug.

We are implementing GeckoView in our project. But we have a problem with clear cookies. Can somebody say, what class clear cookies in geckoview? Urgent! Best Regards

(In reply to Ali Kasimoglu from comment #86) > We are implementing GeckoView in our project. But we have a problem with > clear cookies. > Can somebody say, what class clear cookies in geckoview? Urgent! Best Regards You're more likely to get an answer if you ask on the mobile-firefox-dev mailing list [1], or in the #mobile IRC channel [2]. [1] https://mail.mozilla.org/listinfo/mobile-firefox-dev [2] https://wiki.mozilla.org/IRC

(In reply to Botond Ballo [:botond] from comment #87) > (In reply to Ali Kasimoglu from comment #86) > > We are implementing GeckoView in our project. But we have a problem with > > clear cookies. > > Can somebody say, what class clear cookies in geckoview? Urgent! Best Regards > > You're more likely to get an answer if you ask on the mobile-firefox-dev > mailing list [1], or in the #mobile IRC channel [2]. > > [1] https://mail.mozilla.org/listinfo/mobile-firefox-dev > [2] https://wiki.mozilla.org/IRC thank you