Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TIMOB-4706: Android: Cannot debug an application while using require('some_js_module') #379

Merged
merged 6 commits into from Aug 26, 2011
Merged

TIMOB-4706: Android: Cannot debug an application while using require('some_js_module') #379

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3c25ac4
the debugger now allows us to evaluate javascript strings correctly
marshall Aug 15, 2011
072c9ad
Merge branch 'master' of github.com:appcelerator/titanium_mobile into…
marshall Aug 19, 2011
50f0f49
moving string appends to StringBuilder, added simple unit tests for u…
marshall Aug 19, 2011
3ee97a0
Merge branch 'master' of github.com:appcelerator/titanium_mobile into…
marshall Aug 26, 2011
a7e929b
adding some missing if (DBG)s, changed the interval debug message to a
marshall Aug 26, 2011
53b8773
Merge branch 'master' of github.com:appcelerator/titanium_mobile into…
marshall Aug 26, 2011
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Binary file modified android/titanium/lib/js-debug.jar
View file
Open in desktop
Binary file not shown.
Binary file modified android/titanium/lib/js.jar
View file
Open in desktop
Binary file not shown.
1 change: 1 addition & 0 deletions android/titanium/src/org/appcelerator/titanium/TiC.java
View file
Open in desktop
Expand Up @@ -288,6 +288,7 @@ public class TiC
public static final String PROPERTY_TRANSFORM = "transform";
public static final String PROPERTY_TRUE_HEADING = "trueHeading";
public static final String PROPERTY_TYPE = "type";
public static final String PROPERTY_URI = "uri";
public static final String PROPERTY_URL = "url";
public static final String PROPERTY_USER_LOCATION = "userLocation";
public static final String PROPERTY_VALUE = "value";
Expand Down
105 changes: 58 additions & 47 deletions android/titanium/src/ti/modules/titanium/TitaniumModule.java
View file
Open in desktop
Expand Up @@ -27,6 +27,7 @@
import org.appcelerator.titanium.TiApplication;
import org.appcelerator.titanium.TiBaseActivity;
import org.appcelerator.titanium.TiBlob;
import org.appcelerator.titanium.TiC;
import org.appcelerator.titanium.TiContext;
import org.appcelerator.titanium.TiLaunchActivity;
import org.appcelerator.titanium.io.TiBaseFile;
Expand All @@ -50,7 +51,8 @@
import android.os.Handler;

@Kroll.module @Kroll.topLevel({"Ti", "Titanium"})
public class TitaniumModule extends KrollModule implements TiContext.OnLifecycleEvent, TiContext.OnServiceLifecycleEvent
public class TitaniumModule extends KrollModule
implements TiContext.OnLifecycleEvent, TiContext.OnServiceLifecycleEvent
{
private static final String LCAT = "TitaniumModule";
private static final boolean DBG = TiConfig.LOGD;
Expand All @@ -74,7 +76,7 @@ public TitaniumModule(TiContext tiContext)
@Kroll.getProperty @Kroll.method
public String getUserAgent()
{
return System.getProperties().getProperty("http.agent")+" Titanium/"+getVersion();
return System.getProperties().getProperty("http.agent") + " Titanium/" + getVersion();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StringBuilder for more than one concatenation. Should check for missing http.agent too. I think we're also suppose to support it from tiapp.xml as a user override. Can't remember the key though.

}

@Kroll.getProperty @Kroll.method
Expand Down Expand Up @@ -395,7 +397,8 @@ public String localize(KrollInvocation invocation, Object args[])
}
}

protected KrollModule requireNativeModule(TiContext context, String path) {
protected KrollModule requireNativeModule(TiContext context, String path)
{
Log.d(LCAT, "Attempting to include native module: " + path);
KrollModuleInfo info = KrollModule.getModuleInfo(path);
if (info == null) return null;
Expand All @@ -404,9 +407,9 @@ protected KrollModule requireNativeModule(TiContext context, String path) {
}

@Kroll.method @Kroll.topLevel
public KrollProxy require(KrollInvocation invocation, String path) {

// 1. look for a TiPlus module first
public KrollProxy require(KrollInvocation invocation, String path)
{
// 1. look for a native module first
// 2. then look for a cached module
// 3. then attempt to load from resources
TiContext ctx = invocation.getTiContext().getRootActivity().getTiContext();
Expand All @@ -417,53 +420,61 @@ public KrollProxy require(KrollInvocation invocation, String path) {
return module;
}

// NOTE: commonjs modules load absolute to root in Titanium
String fileUrl = "app://"+path+".js";
// NOTE: CommonJS modules load absolute to app:// in Titanium
String fileUrl = "app://" + path + ".js";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use StringBuilder for more than 1 concat. Also, don't we have a constant for "app://" somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we have it in constant form -- I'll use that + StringBuilder

TiBaseFile tbf = TiFileFactory.createTitaniumFile(ctx, new String[]{ fileUrl }, false);
if (tbf!=null)
{
try
{
TiBlob blob = (TiBlob)tbf.read();
if (blob == null) {
Log.e(LCAT, "Couldn't read required file: " + fileUrl);
return null;
}
if (tbf == null) {
//the spec says we are required to throw an exception
Context.reportError("Couldn't find module: " + path);
return null;
}

// create the common js exporter
KrollProxy proxy = new KrollProxy(ctx);
StringBuilder buf = new StringBuilder();
buf.append("(function(exports){");
buf.append(blob.getText());
buf.append("return exports;");
buf.append("})({})");
Scriptable result = (Scriptable)ctx.evalJS(buf.toString());
// common js modules export all functions/properties as
// properties of the special export object provided
for (Object key : result.getIds())
{
String propName = key.toString();
Scriptable propValue = (Scriptable)result.get(propName,result);
proxy.setProperty(propName, propValue);
}
// spec says you must have a read-only id property - we don't
// currently support readonly in kroll so this is probably OK for now
proxy.setProperty("id", path);
// uri is optional but we point it to where we loaded it
proxy.setProperty("uri",fileUrl);
return proxy;
Log.d(LCAT, "Attempting to include JS module: " + tbf.nativePath());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This log statement should be upgraded to INFO or wrapped in if (DBG){}

try {
TiBlob blob = (TiBlob) tbf.read();
if (blob == null) {
Log.e(LCAT, "Couldn't read required file: " + fileUrl);
return null;
}
catch(Exception ex)
{
Log.e(LCAT,"Error loading module named: "+path,ex);
Context.throwAsScriptRuntimeEx(ex);

// TODO: we need to switch to the Rhino native require()
// implementation, but in the meantime this will have to do

// create the CommonJS exporter
KrollProxy proxy = new KrollProxy(ctx);
StringBuilder buf = new StringBuilder();
buf.append("(function(exports){");
buf.append(blob.getText());
buf.append("return exports;");
buf.append("})({})");

Object result = ctx.evalJS(buf.toString());

if (!(result instanceof Scriptable)) {
Context.throwAsScriptRuntimeEx(new Exception("Module did not correctly return an exports object: " + path + ", result: " + result));
return null;
}
}

//the spec says we are required to throw an exception
Context.reportError("couldn't find module: "+path);
return null;
Scriptable exports = (Scriptable) result;
// CommonJS modules export all functions/properties as
// properties of the special exports object provided
for (Object key : exports.getIds()) {
String propName = key.toString();
Scriptable propValue = (Scriptable) exports.get(propName, exports);
proxy.setProperty(propName, propValue);
}

// spec says you must have a read-only id property - we don't
// currently support readonly in kroll so this is probably OK for now
proxy.setProperty(TiC.PROPERTY_ID, path);
// uri is optional but we point it to where we loaded it
proxy.setProperty(TiC.PROPERTY_URI, fileUrl);
return proxy;
} catch (Exception ex) {
Log.e(LCAT, "Error loading module named: " + path, ex);
Context.throwAsScriptRuntimeEx(ex);
return null;
}
}

@Kroll.method
Expand Down