Skip to content

Commit

Permalink
Refactor Android printing code to make it more testable.
Browse files Browse the repository at this point in the history
* Move printing logic from Tab to TabBase (i.e. to upstream), and
  also in the relevant files tab_android.*, TabPrinter.java.
* Remove obsolete Android printing feature detection code.
* Move PrintingControllerFactory logic into PrintingControllerImpl.
* Create a new PrintingControllerFactory, so the clients have a
ligher weight creation process (5-6 lines to 1).
* Instead of depending on Context to create a PrintManager, depend
  on an interface, namely PrintManagerDelegate.
* Remove setErrorText (move the logic inside factory).
* Remove the hardcoded default file name (use Printable#getTitle)
  instead.

BUG=315229

Review URL: https://codereview.chromium.org/63483007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@236256 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
cimamoglu@chromium.org committed Nov 20, 2013
1 parent c665ede commit 90fba6c
Show file tree
Hide file tree
Showing 12 changed files with 245 additions and 118 deletions.
12 changes: 11 additions & 1 deletion chrome/android/java/src/org/chromium/chrome/browser/TabBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,6 @@ protected Context getApplicationContext() {
}

/**
*
* @return The infobar container.
*/
public final InfoBarContainer getInfoBarContainer() {
Expand All @@ -322,6 +321,16 @@ public final InfoBarContainer getInfoBarContainer() {
*/
protected abstract AutoLoginProcessor createAutoLoginProcessor();

/**
* Prints the current page.
*
* @return Whether the printing process is started successfully.
**/
public boolean print() {
assert mNativeTabAndroid != 0;
return nativePrint(mNativeTabAndroid);
}

/**
* Reloads the current page content if it is a {@link ContentView}.
*/
Expand Down Expand Up @@ -771,4 +780,5 @@ private native void nativeInitWebContents(int nativeTabAndroid, boolean incognit
private native int nativeGetSecurityLevel(int nativeTabAndroid);
private native void nativeSetActiveNavigationEntryTitleForUrl(int nativeTabAndroid, String url,
String title);
private native boolean nativePrint(int nativeTabAndroid);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright 2013 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.chrome.browser.printing;

import android.app.Activity;
import android.content.Context;
import android.print.PrintManager;

import org.chromium.chrome.R;
import org.chromium.printing.PrintManagerDelegateImpl;
import org.chromium.printing.PrintingController;
import org.chromium.printing.PrintingControllerImpl;

/**
* Creates a {@link PrintingControllerImpl}.
*
* Also, sets the default title of {@link TabPrinter}.
*/
public class PrintingControllerFactory {
public static PrintingController create(Activity activity) {
if (PrintingControllerImpl.isPrintingSupported()) {
String defaultJobTitle = activity.getResources().getString(R.string.menu_print);
TabPrinter.setDefaultTitle(defaultJobTitle);

PrintManager printManager =
(PrintManager) activity.getSystemService(Context.PRINT_SERVICE);
String errorText = activity.getResources().getString(R.string.error_printing_failed);
return PrintingControllerImpl.create(
new PrintManagerDelegateImpl(printManager), errorText);
}
return null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright 2013 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.chrome.browser.printing;

import android.text.TextUtils;

import org.chromium.chrome.browser.TabBase;
import org.chromium.printing.Printable;

import java.lang.ref.WeakReference;

/**
* Wraps printing related functionality of a {@link TabBase} object.
*
* This class doesn't have any lifetime expectations with regards to Tab, since we keep a weak
* reference.
*/
public class TabPrinter implements Printable {
private static String sDefaultTitle;

private final WeakReference<TabBase> mTab;

public TabPrinter(TabBase tab) {
mTab = new WeakReference<TabBase>(tab);
}

public static void setDefaultTitle(String defaultTitle) {
sDefaultTitle = defaultTitle;
}

@Override
public boolean print() {
TabBase tab = mTab.get();
return tab != null && tab.print();
}

@Override
public String getTitle() {
TabBase tab = mTab.get();
if (tab == null) return sDefaultTitle;

String title = tab.getTitle();
if (!TextUtils.isEmpty(title)) return title;

String url = tab.getUrl();
if (!TextUtils.isEmpty(url)) return url;

return sDefaultTitle;
}
}
15 changes: 15 additions & 0 deletions chrome/browser/android/tab_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "chrome/browser/predictors/resource_prefetch_predictor_factory.h"
#include "chrome/browser/predictors/resource_prefetch_predictor_tab_helper.h"
#include "chrome/browser/prerender/prerender_tab_helper.h"
#include "chrome/browser/printing/print_view_manager_basic.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_android.h"
#include "chrome/browser/sessions/session_tab_helper.h"
Expand Down Expand Up @@ -399,6 +400,20 @@ void TabAndroid::SetActiveNavigationEntryTitleForUrl(JNIEnv* env,
entry->SetTitle(title);
}

bool TabAndroid::Print(JNIEnv* env, jobject obj) {
if (!web_contents())
return false;

printing::PrintViewManagerBasic::CreateForWebContents(web_contents());
printing::PrintViewManagerBasic* print_view_manager =
printing::PrintViewManagerBasic::FromWebContents(web_contents());
if (print_view_manager == NULL)
return false;

print_view_manager->PrintNow();
return true;
}

bool TabAndroid::RegisterTabAndroid(JNIEnv* env) {
return RegisterNativesImpl(env);
}
2 changes: 2 additions & 0 deletions chrome/browser/android/tab_android.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ class TabAndroid : public CoreTabHelperDelegate,
jstring jurl,
jstring jtitle);

bool Print(JNIEnv* env, jobject obj);

protected:
virtual ~TabAndroid();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2013 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.printing;

import android.print.PrintAttributes;
import android.print.PrintDocumentAdapter;

/**
* Defines an interface for the Android system printing service, for easier testing.
* We can't simply extend from {@link android.print.PrintManager}, since it's a final class.
*/
public interface PrintManagerDelegate {

/**
* Same as {@link android.print.PrintManager#print}, except this doesn't return a
* {@link android.print.PrintJob} since the clients don't need it.
*/
void print(String printJobName,
PrintDocumentAdapter documentAdapter,
PrintAttributes attributes);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2013 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.printing;

import android.print.PrintAttributes;
import android.print.PrintDocumentAdapter;
import android.print.PrintManager;

/**
* An implementation of {@link PrintManagerDelegate} using the Android framework print manager.
*/
public class PrintManagerDelegateImpl implements PrintManagerDelegate {
private final PrintManager mPrintManager;

public PrintManagerDelegateImpl(PrintManager printManager) {
mPrintManager = printManager;
}

@Override
public void print(String printJobName, PrintDocumentAdapter documentAdapter,
PrintAttributes attributes) {
mPrintManager.print(printJobName, documentAdapter, attributes);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@
import org.chromium.base.JNINamespace;
import org.chromium.base.ThreadUtils;

import android.content.Context;

import android.util.Log;
import android.util.SparseArray;

/**
Expand All @@ -21,25 +18,14 @@
@JNINamespace("printing")
public class PrintingContext implements PrintingContextInterface {

private static final String TAG = "PrintingContext";

/** Whether the framework supports printing. */
public static final boolean sIsPrintingAvailable = isPrintingAvailable();

/**
* The full class name of the print manager used to test whether printing functionality is
* available.
*/
private static final String PRINT_MANAGER_CLASS_NAME = "android.print.PrintManager";

/**
* Mapping from a file descriptor (as originally provided from
* {@link PrintDocumentAdapter#onWrite}) to a PrintingContext.
*
* This is static because a static method of the native code (inside PrintingContextAndroid)
* needs to find Java PrintingContext class corresponding to a file descriptor.
**/
private static final SparseArray<PrintingContext> sPrintingContextMap =
private static final SparseArray<PrintingContext> PRINTING_CONTEXT_MAP =
new SparseArray<PrintingContext>();

/** The controller this object interacts with, which in turn communicates with the framework. */
Expand All @@ -48,27 +34,13 @@ public class PrintingContext implements PrintingContextInterface {
/** The pointer to the native PrintingContextAndroid object. */
private final int mNativeObject;

private PrintingContext(Context context, int ptr) {
mController = PrintingControllerFactory.getPrintingController(context);
private PrintingContext(int ptr) {
mController = PrintingControllerImpl.getInstance();
mNativeObject = ptr;
}

/**
* @return Whether printing is supported by the platform.
*/
private static boolean isPrintingAvailable() {
// TODO(cimamoglu): Get rid of reflection once Build.VERSION_CODES.KEY_LIME_PIE is fixed.
try {
Class.forName(PRINT_MANAGER_CLASS_NAME);
} catch (ClassNotFoundException e) {
Log.d(TAG, "PrintManager not found on device");
return false;
}
return true;
}

/**
* Updates sPrintingContextMap to map from the file descriptor to this object.
* Updates PRINTING_CONTEXT_MAP to map from the file descriptor to this object.
* @param fileDescriptor The file descriptor passed down from
* {@link PrintDocumentAdapter#onWrite}.
* @param delete If true, delete the entry (if it exists). If false, add it to the map.
Expand All @@ -77,9 +49,9 @@ private static boolean isPrintingAvailable() {
public void updatePrintingContextMap(int fileDescriptor, boolean delete) {
ThreadUtils.assertOnUiThread();
if (delete) {
sPrintingContextMap.remove(fileDescriptor);
PRINTING_CONTEXT_MAP.remove(fileDescriptor);
} else {
sPrintingContextMap.put(fileDescriptor, this);
PRINTING_CONTEXT_MAP.put(fileDescriptor, this);
}
}

Expand All @@ -94,9 +66,9 @@ public void askUserForSettingsReply(boolean success) {
}

@CalledByNative
public static PrintingContext create(Context context, int nativeObjectPointer) {
public static PrintingContext create(int nativeObjectPointer) {
ThreadUtils.assertOnUiThread();
return new PrintingContext(context, nativeObjectPointer);
return new PrintingContext(nativeObjectPointer);
}

@CalledByNative
Expand Down Expand Up @@ -127,11 +99,11 @@ public int getHeight() {
public static void pdfWritingDone(int fd, boolean success) {
ThreadUtils.assertOnUiThread();
// TODO(cimamoglu): Do something when fd == -1.
if (sPrintingContextMap.get(fd) != null) {
if (PRINTING_CONTEXT_MAP.get(fd) != null) {
ThreadUtils.assertOnUiThread();
PrintingContext printingContext = sPrintingContextMap.get(fd);
PrintingContext printingContext = PRINTING_CONTEXT_MAP.get(fd);
printingContext.mController.pdfWritingDone(success);
sPrintingContextMap.remove(fd);
PRINTING_CONTEXT_MAP.remove(fd);
}
}

Expand All @@ -147,7 +119,7 @@ public void pageCountEstimationDone(final int maxPages) {
// If the printing dialog has already finished, tell Chromium that operation is cancelled.
if (mController.hasPrintingFinished()) {
// NOTE: We don't call nativeAskUserForSettingsReply (hence Chromium callback in
// AskUserForSettings callback) twice. See PrintingControllerImpl#onFinish
// AskUserForSettings callback) twice. See {@link PrintingControllerImpl#onFinish}
// for more explanation.
nativeAskUserForSettingsReply(mNativeObject, false);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,22 +62,14 @@ public interface PrintingController {
void pageCountEstimationDone(final int maxPages);

/**
* Sets PrintingContext.
* Sets PrintingContext currently associated with the controller.
*
* This needs to be called after PrintingContext object is created. Firstly its native
* counterpart is created, and then the Java. PrintingController implementation
* needs this to interact with the native side, since JNI is built on PrintingContext.
**/
void setPrintingContext(final PrintingContextInterface printingContext);

/**
* TODO(cimamoglu): Remove errorText stuff once KitKat is public and we can move this code.
* @param errorText The error message to be shown to user in case something goes wrong in PDF
* generation in Chromium. We pass it here as a string because this folder
* cannot use resources directly (or any other Clank code).
*/
void setErrorText(final String errorText);

/**
* @return Whether a complete PDF generation cycle inside Chromium has been completed.
*/
Expand Down

This file was deleted.

Loading

0 comments on commit 90fba6c

Please sign in to comment.