Skip to content
This repository has been archived by the owner on Jul 24, 2023. It is now read-only.

Make connection to CodeGeneratorService lazy. #509

Expand Up @@ -16,20 +16,18 @@
package com.google.blockly.android;

import android.content.Context;
import android.content.Intent;
import android.content.SharedPreferences;
import android.content.res.AssetManager;
import android.os.Bundle;
import android.preference.PreferenceManager;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.v4.app.Fragment;
import android.support.v4.app.FragmentManager;
import android.support.v4.view.GravityCompat;
import android.support.v4.widget.DrawerLayout;
import android.support.v7.app.ActionBar;
import android.support.v7.app.ActionBarDrawerToggle;
import android.support.v7.app.AppCompatActivity;
import android.support.v4.app.Fragment;
import android.util.Log;
import android.view.Gravity;
import android.view.Menu;
Expand All @@ -41,20 +39,20 @@

import com.google.blockly.android.clipboard.BlockClipDataHelper;
import com.google.blockly.android.clipboard.SingleMimeTypeClipDataHelper;
import com.google.blockly.android.codegen.CodeGeneratorService;
import com.google.blockly.android.codegen.CodeGenerationRequest;
import com.google.blockly.android.codegen.CodeGeneratorManager;
import com.google.blockly.android.control.BlocklyController;
import com.google.blockly.android.ui.BlockViewFactory;
import com.google.blockly.android.ui.BlocklyUnifiedWorkspace;
import com.google.blockly.android.ui.DeleteVariableDialog;
import com.google.blockly.android.ui.NameVariableDialog;
import com.google.blockly.android.ui.TrashCanView;
import com.google.blockly.android.ui.WorkspaceHelper;
import com.google.blockly.android.ui.BlocklyUnifiedWorkspace;
import com.google.blockly.android.codegen.CodeGenerationRequest;
import com.google.blockly.model.Block;
import com.google.blockly.model.BlockFactory;
import com.google.blockly.model.BlocklySerializerException;
import com.google.blockly.model.Workspace;
import com.google.blockly.utils.StringOutputStream;

import java.io.FileNotFoundException;
import java.io.IOException;
Expand Down Expand Up @@ -747,11 +745,27 @@ public void onClick(View v) {
* @see #getCodeGenerationCallback()
*/
protected void onRunCode() {
mCodeGeneratorManager.requestCodeGeneration(
getBlockDefinitionsJsonPaths(),
getGeneratorsJsPaths(),
mWorkspaceFragment.getWorkspace(),
getCodeGenerationCallback());
Workspace workspace = mWorkspaceFragment.getWorkspace();
if (workspace.hasBlocks()) {
try {
final StringOutputStream serialized = new StringOutputStream();
workspace.serializeToXml(serialized);
CodeGenerationRequest codeGenerationRequest =
new CodeGenerationRequest(
serialized.toString(),
getCodeGenerationCallback(),
getBlockDefinitionsJsonPaths(),
getGeneratorsJsPaths());
mCodeGeneratorManager.requestCodeGeneration(codeGenerationRequest);
} catch (BlocklySerializerException e) {
Log.wtf(TAG, e);
Toast.makeText(this, "Code generation failed.",
Toast.LENGTH_LONG).show();

}
} else {
Log.i(TAG, "No blocks in workspace. Skipping run request.");
}
Copy link
Contributor

@AnmAtAnm AnmAtAnm Feb 2, 2017

Choose a reason for hiding this comment

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

Because this is going to have merge conflicts with some upcoming changes, I've updated the PR to target the toolbox-refactor branch. I'd appreciate if I can have write access to this branch to fix the upcoming conflict.

Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I've done this, but let me know if there are still issues (the merge conflicts were all imports, so trivial to resolve).

}

/**
Expand Down
Expand Up @@ -5,92 +5,87 @@
import android.content.Intent;
import android.content.ServiceConnection;
import android.os.IBinder;
import android.util.Log;
import android.widget.Toast;

import com.google.blockly.android.codegen.CodeGenerationRequest.CodeGeneratorCallback;
import com.google.blockly.model.BlocklySerializerException;
import com.google.blockly.model.Workspace;
import com.google.blockly.utils.StringOutputStream;

import java.util.List;
import java.util.LinkedList;
import java.util.Queue;

/**
* Client Side class responsible for connecting to the {@link CodeGeneratorService}.
*
* A connection to the service is only made the first time code generation is requested.
*/
public class CodeGeneratorManager {
private static final String TAG = "CodeGeneratorManager";

private final Context mContext;
private final ServiceConnection mCodeGenerationConnection;
private final Queue<CodeGenerationRequest> mStoredRequests;

private ServiceConnection mCodeGenerationConnection;
private CodeGeneratorService mGeneratorService;

public CodeGeneratorManager(Context context) {
this.mContext = context;

this.mCodeGenerationConnection = new ServiceConnection() {
@Override
public void onServiceConnected(ComponentName className, IBinder binder) {
mGeneratorService =
((CodeGeneratorService.CodeGeneratorBinder) binder).getService();
}

@Override
public void onServiceDisconnected(ComponentName arg0) {
mGeneratorService = null;
}
};
this.mStoredRequests = new LinkedList<>();
}

/**
* Resumes the underlying code generation connection.
*/
public void onResume() {
Intent intent = new Intent(mContext, CodeGeneratorService.class);
mContext.bindService(intent, mCodeGenerationConnection, Context.BIND_AUTO_CREATE);
if (mCodeGenerationConnection != null) {
mContext.bindService(intent, mCodeGenerationConnection, Context.BIND_AUTO_CREATE);
Copy link
Contributor

Choose a reason for hiding this comment

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

To be clear, it is this bind that could be lazy, and is the point of issue #37. We should wait until the first requestCodeGeneration() call, since it is entirely possible Blockly is used without code generation, and a full background WebView is fairly heavyweight. Scratch Blocks is an example of this sort of usage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense. I've made an update so that the bind is lazy (rather than creation of the connection).

}
}

/**
* Pauses the underlying code generation connection.
*/
public void onPause() {
mContext.unbindService(mCodeGenerationConnection);
if (mCodeGenerationConnection != null) {
mContext.unbindService(mCodeGenerationConnection);
}
}

/**
* Calls the Service to request code generation for the workspace passed in.
*
* @param workspace the workspace to request code generation for.
* @param codeGenerationCallback the callback to call with the generated code.
* @param codeGenerationRequest the request to generate code.
*/
public void requestCodeGeneration(List<String> blockDefinitionsJsonPaths,
List<String> generatorsJsPaths,
Workspace workspace,
CodeGeneratorCallback codeGenerationCallback) {

if (workspace.hasBlocks()) {
if (isBound()) {
try {
final StringOutputStream serialized = new StringOutputStream();
workspace.serializeToXml(serialized);

mGeneratorService.requestCodeGeneration(
new CodeGenerationRequest(
serialized.toString(),
codeGenerationCallback,
blockDefinitionsJsonPaths,
generatorsJsPaths)
);
} catch (BlocklySerializerException e) {
Log.wtf(TAG, e);
Toast.makeText(mContext, "Code generation failed.",
Toast.LENGTH_LONG).show();

}
} else {
Log.i(TAG, "Generator not bound to service. Skipping run request.");
}
public void requestCodeGeneration(CodeGenerationRequest codeGenerationRequest) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@NotNull CodeGenerationRequest codeGenerationRequest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nulls are already handled in the code, but I'll add a null check here so that that's clearer.

if (isBound()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add if(!mResumed) { Log.w(TAG, "Code generation called while paused. Request ignored."); return;}

Copy link
Contributor

Choose a reason for hiding this comment

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

Check resumed state first.

executeCodeGenerationRequest(codeGenerationRequest);
} else {
Log.i(TAG, "No blocks in workspace. Skipping run request.");
mStoredRequests.add(codeGenerationRequest);
connectToService();
}
}

private boolean isBound() {
return mGeneratorService != null;
}

private void connectToService() {
this.mCodeGenerationConnection = new ServiceConnection() {
@Override
public void onServiceConnected(ComponentName className, IBinder binder) {
mGeneratorService = ((CodeGeneratorService.CodeGeneratorBinder) binder).getService();
while (!mStoredRequests.isEmpty()) {
executeCodeGenerationRequest(mStoredRequests.poll());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've used poll() rather than remove() here, as the private execute method can handle nulls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've used poll() rather than remove() here, as executeCodeGenerationRequest can handle nulls.

}
}

@Override
public void onServiceDisconnected(ComponentName arg0) {
mGeneratorService = null;
}
};

}

private void executeCodeGenerationRequest(CodeGenerationRequest request) {
if (request != null) {
mGeneratorService.requestCodeGeneration(request);
}
}
}