Throw exception when initialization failed.

Throw exception when initialization failed.

Previously the initialization return code was ignored, it will lead us
crash later. We need a way to exit and maybe show error message if something
wrong during initialization.

Using exception might better than returning the result code since the
initialization code could be called from constructor and initialization
error is an exception at most of time.

In activity, the exception is caught, then call finish() to exit.

In tests, the exception is wrapped with Error and threw out.

BUG=b/7705055


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=176975

[michaelbai, 2012-12-21 20:02:27]

Hi all,

This CL is not complete, there are somewhere need to catch the exception. Can we
agree to use exception then I finish this CL? It seemed that we haven't used
exception in our code.

[joth, 2012-12-21 21:53:12]

I think an unchecked exception would be a better fit here, and more consistent
with how we deal with the library load failure. (Added Ted as he wrote that)

https://codereview.chromium.org/11567061/diff/1/android_webview/java/src/org/...
File android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java
(right):

https://codereview.chromium.org/11567061/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java:54:
// TODO: How do we handle the exception here?
throw new Error("Cannot initialize WebView", e);

https://codereview.chromium.org/11567061/diff/1/content/public/android/java/s...
File
content/public/android/java/src/org/chromium/content/browser/AndroidBrowserProcessInitException.java
(right):

https://codereview.chromium.org/11567061/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/AndroidBrowserProcessInitException.java:7:
public class AndroidBrowserProcessInitException extends Exception {
consider having this extend Error, like UnsatisfiedLinkError does.

That way you don't have to add throws clauses all over the place, and we can
instead have a top-level catch for both these errors in the init path, and gives
the same "Either app not installed properly, or your phone is not supported"
bail out.

[michaelbai, 2013-01-04 23:38:31]

Talked to Ted, we agreed to use Exception because Error is not required to be
caught and easy to miss, so,

- UnsatisfiedLinkErrors were replaced with exception
- Renamed AndroidBrowserProcessInitException to ProcessInitException, so it
could be used in all types of process.


PTAL

[michaelbai, 2013-01-04 23:40:53]

joi@ please help to review 	 content/public/common/result_codes.h

[Jói, 2013-01-07 12:37:16]

LGTM for content/public/common.

On Fri, Jan 4, 2013 at 11:40 PM,  <michaelbai@chromium.org> wrote:
> joi@ please help to review       content/public/common/result_codes.h
>
> https://codereview.chromium.org/11567061/

[Ted C, 2013-01-07 18:38:45]

https://codereview.chromium.org/11567061/diff/5001/android_webview/java/src/o...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/11567061/diff/5001/android_webview/java/src/o...
android_webview/java/src/org/chromium/android_webview/AwContents.java:279: 
I think this needs to be indented 8 more because of the long parameters lines
above.

https://codereview.chromium.org/11567061/diff/5001/android_webview/javatests/...
File
android_webview/javatests/src/org/chromium/android_webview/test/AndroidWebViewTestBase.java
(right):

https://codereview.chromium.org/11567061/diff/5001/android_webview/javatests/...
android_webview/javatests/src/org/chromium/android_webview/test/AndroidWebViewTestBase.java:219:

it doesn't actually look like it throws Exception anymore, are these needed? 
Also it would be better to scope this to be more fine grained than general
exception.

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
File content/public/android/java/src/org/chromium/content/app/LibraryLoader.java
(right):

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/app/LibraryLoader.java:163:

is that supposed to be common instead of comment?

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/browser/AndroidBrowserProcess.java
(right):

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/AndroidBrowserProcess.java:163:

should this just != 0?

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/AndroidBrowserProcess.java:164:

in java, braces are always required unless the conditional and statement can all
fit on one line (looks like this can, so you should be able to just move this up
with the result check):

http://source.android.com/source/code-style.html#use-standard-brace-style

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/browser/ContentView.java
(right):

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/ContentView.java:119:

+8

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/ContentView.java:138:

+8 and the two below

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/browser/JellyBeanContentView.java
(right):

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/JellyBeanContentView.java:22:

+8

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/common/ProcessInitException.java
(right):

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/common/ProcessInitException.java:7:
public class ProcessInitException extends Exception {
class level javadoc

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/common/ProcessInitException.java:8:
private int mErrorCode = 0;
this file is indented c style, but needs to be indented 4 spaces each instead of
2

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/common/ProcessInitException.java:10:
public ProcessInitException(int errorCode) {
what does this javadoc represent?  all public methods should have javadocs as
well, so I guess explain it there.

[michaelbai, 2013-01-07 23:22:07]

Hi Ted, PTAL

https://codereview.chromium.org/11567061/diff/5001/android_webview/java/src/o...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/11567061/diff/5001/android_webview/java/src/o...
android_webview/java/src/org/chromium/android_webview/AwContents.java:279: 
On 2013/01/07 18:38:46, Ted C wrote:
> I think this needs to be indented 8 more because of the long parameters lines
> above.

Done.

https://codereview.chromium.org/11567061/diff/5001/android_webview/javatests/...
File
android_webview/javatests/src/org/chromium/android_webview/test/AndroidWebViewTestBase.java
(right):

https://codereview.chromium.org/11567061/diff/5001/android_webview/javatests/...
android_webview/javatests/src/org/chromium/android_webview/test/AndroidWebViewTestBase.java:219:

It throws ProcessInitException because of AwContents constructor.

Changed Exception to ProcessInitException.

On 2013/01/07 18:38:46, Ted C wrote:
> it doesn't actually look like it throws Exception anymore, are these needed? 
> Also it would be better to scope this to be more fine grained than general
> exception.

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
File content/public/android/java/src/org/chromium/content/app/LibraryLoader.java
(right):

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/app/LibraryLoader.java:163:

On 2013/01/07 18:38:46, Ted C wrote:
> is that supposed to be common instead of comment?

Done.

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/browser/AndroidBrowserProcess.java
(right):

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/AndroidBrowserProcess.java:163:

Actually the negative value means success here, see my change in
chrome_main_delegate_android.cc and content_main_runner.cc
RunNamedProcessTypeMain(...)

On 2013/01/07 18:38:46, Ted C wrote:
> should this just != 0?

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/AndroidBrowserProcess.java:164:

On 2013/01/07 18:38:46, Ted C wrote:
> in java, braces are always required unless the conditional and statement can
all
> fit on one line (looks like this can, so you should be able to just move this
up
> with the result check):
> 
> http://source.android.com/source/code-style.html#use-standard-brace-style

Done.

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/browser/ContentView.java
(right):

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/ContentView.java:119:

On 2013/01/07 18:38:46, Ted C wrote:
> +8

Done.

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/ContentView.java:138:

On 2013/01/07 18:38:46, Ted C wrote:
> +8 and the two below

Done.

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/browser/JellyBeanContentView.java
(right):

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/JellyBeanContentView.java:22:

On 2013/01/07 18:38:46, Ted C wrote:
> +8

Done.

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/common/ProcessInitException.java
(right):

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/common/ProcessInitException.java:7:
public class ProcessInitException extends Exception {
On 2013/01/07 18:38:46, Ted C wrote:
> class level javadoc

Done.

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/common/ProcessInitException.java:8:
private int mErrorCode = 0;
On 2013/01/07 18:38:46, Ted C wrote:
> this file is indented c style, but needs to be indented 4 spaces each instead
of
> 2

Done.

https://codereview.chromium.org/11567061/diff/5001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/common/ProcessInitException.java:10:
public ProcessInitException(int errorCode) {
On 2013/01/07 18:38:46, Ted C wrote:
> what does this javadoc represent?  all public methods should have javadocs as
> well, so I guess explain it there.

Done.

[Yaron, 2013-01-08 00:54:21]

I'm with joth on this one. This seems like it should be an Error or at least
RuntimeException and not a checked exception. There's too much code that's just
passing the buck and it's a catastrophic failure that most callers can't recover
from and yet are forced to be exposed to (e.g. creating an AwContents or
ContentView{,Core}). If most consumers don't care, that should favour unchecked.
Finally, this is comparable to UnsatisfiedLinkError which is an error, not
checked exception.

https://codereview.chromium.org/11567061/diff/21001/content/app/android/libra...
File content/app/android/library_loader_hooks.cc (right):

https://codereview.chromium.org/11567061/diff/21001/content/app/android/libra...
content/app/android/library_loader_hooks.cc:89: return 0;
This should probably be RESULT_CODE_NORMAL_EXIT to indicate difference from
above.

https://codereview.chromium.org/11567061/diff/21001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/common/ProcessInitException.java
(right):

https://codereview.chromium.org/11567061/diff/21001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/common/ProcessInitException.java:18:
super();
Why is this necessary? Please remove.

https://codereview.chromium.org/11567061/diff/21001/content/public/common/res...
File content/public/common/result_codes.h (right):

https://codereview.chromium.org/11567061/diff/21001/content/public/common/res...
content/public/common/result_codes.h:35: RESULT_CODE_FAILED_TO_REG_JNI = 4,
*_REGISTER_*

[michaelbai, 2013-01-08 18:06:48]

This Exception is what we want to catch, to make it a checked Exception will
prevent people from missing to catch it.

I wrapped in a Error in AndroidWebview related code because Android API can be
changed.

You could also see we missed to check UnsatisfiedLinkError (replace with
ProcessInitException) in many places. https://gerrit-int.chromium.org/#/c/30443

https://codereview.chromium.org/11567061/diff/21001/content/app/android/libra...
File content/app/android/library_loader_hooks.cc (right):

https://codereview.chromium.org/11567061/diff/21001/content/app/android/libra...
content/app/android/library_loader_hooks.cc:89: return 0;
RESULT_CODE_NORMAL_EXIT means the process exit normally, it might not suitable
to be used here.

On 2013/01/08 00:54:22, Yaron wrote:
> This should probably be RESULT_CODE_NORMAL_EXIT to indicate difference from
> above.

https://codereview.chromium.org/11567061/diff/21001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/common/ProcessInitException.java
(right):

https://codereview.chromium.org/11567061/diff/21001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/common/ProcessInitException.java:18:
super();
On 2013/01/08 00:54:22, Yaron wrote:
> Why is this necessary? Please remove.

Done.

https://codereview.chromium.org/11567061/diff/21001/content/public/common/res...
File content/public/common/result_codes.h (right):

https://codereview.chromium.org/11567061/diff/21001/content/public/common/res...
content/public/common/result_codes.h:35: RESULT_CODE_FAILED_TO_REG_JNI = 4,
On 2013/01/08 00:54:22, Yaron wrote:
> *_REGISTER_*

Done.

[michaelbai, 2013-01-08 20:19:03]

Talked to Yaron,

removed unnecessary process initialization in ContentViewCore's constructor as
process should be initialized before creating instance of ContentViewCore.

PTAL

[joth, 2013-01-08 21:20:45]

https://codereview.chromium.org/11567061/diff/28001/android_webview/java/src/...
File android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java
(right):

https://codereview.chromium.org/11567061/diff/28001/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java:55:
throw new Error("Cannot initialize WebView", e);
Unfortunately ThreadUtils.runOnUiThreadBlocking does not propogate Errors back
to the caller, only exceptions.
Coud you update ThreadUtils.runOnUiThreadBlocking to catch Throwable instead of
Exception? (and pass it back through) or just create a RuntimeException here
(maybe more reasonable).

https://codereview.chromium.org/11567061/diff/28001/content/app/android/libra...
File content/app/android/library_loader_hooks.cc (right):

https://codereview.chromium.org/11567061/diff/28001/content/app/android/libra...
content/app/android/library_loader_hooks.cc:39: jobjectArray init_command_line)
{
indent wrong.

https://codereview.chromium.org/11567061/diff/28001/content/app/android/libra...
content/app/android/library_loader_hooks.cc:87: return
RESULT_CODE_FAILED_TO_REGISTER_JNI;
I don't think it's adding much to reuse result_codes.h for these. It seems the
other result codes would be impossible to return here, and conversely the
FAILED_TO_REGSITER_JNI is android only cannot happen subsequent to this point.

just returning a boolean and then on the java side include a message in the
exception (need a new c'tor overload) "JNI registration error" seems as good. we
can define exception.getErrorCode() == -1 to mean other or some such thing

https://codereview.chromium.org/11567061/diff/28001/content/public/android/ja...
File content/public/android/java/src/org/chromium/content/app/LibraryLoader.java
(right):

https://codereview.chromium.org/11567061/diff/28001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/app/LibraryLoader.java:61:
public static void loadAndInitSync() throws ProcessInitException {
could you  just delete this method? it's finally not called anywhere any more

https://codereview.chromium.org/11567061/diff/28001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/app/LibraryLoader.java:118:
Log.e(TAG, "error calling nativeLibraryLoadedOnMainThread");
may as well log the code here.

https://codereview.chromium.org/11567061/diff/28001/content/public/common/res...
File content/public/common/result_codes.h (right):

https://codereview.chromium.org/11567061/diff/28001/content/public/common/res...
content/public/common/result_codes.h:12: // c) the exception value that causes a
process to terminate.
this comment will need updating if we do add this error in here (but see other
comments)

[michaelbai, 2013-01-09 00:14:00]

Hi Joth,

Please see my comment for alternative way to handle this.

https://codereview.chromium.org/11567061/diff/28001/android_webview/java/src/...
File android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java
(right):

https://codereview.chromium.org/11567061/diff/28001/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java:55:
throw new Error("Cannot initialize WebView", e);
Used RuntimeException.


On 2013/01/08 21:20:45, joth wrote:
> Unfortunately ThreadUtils.runOnUiThreadBlocking does not propogate Errors back
> to the caller, only exceptions.
> Coud you update ThreadUtils.runOnUiThreadBlocking to catch Throwable instead
of
> Exception? (and pass it back through) or just create a RuntimeException here
> (maybe more reasonable).
> 
>

https://codereview.chromium.org/11567061/diff/28001/content/app/android/libra...
File content/app/android/library_loader_hooks.cc (right):

https://codereview.chromium.org/11567061/diff/28001/content/app/android/libra...
content/app/android/library_loader_hooks.cc:87: return
RESULT_CODE_FAILED_TO_REGISTER_JNI;
I think it is better for user to find the error in one place. If we are
concerned about the change here, I can revert this file and add a native method
like
 int nativeGetFailedToRegisterJNIResultCode(). 
and add it in Java side. Sounds better?


On 2013/01/08 21:20:45, joth wrote:
> I don't think it's adding much to reuse result_codes.h for these. It seems the
> other result codes would be impossible to return here, and conversely the
> FAILED_TO_REGSITER_JNI is android only cannot happen subsequent to this point.
> 
> just returning a boolean and then on the java side include a message in the
> exception (need a new c'tor overload) "JNI registration error" seems as good.
we
> can define exception.getErrorCode() == -1 to mean other or some such thing

https://codereview.chromium.org/11567061/diff/28001/content/public/android/ja...
File content/public/android/java/src/org/chromium/content/app/LibraryLoader.java
(right):

https://codereview.chromium.org/11567061/diff/28001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/app/LibraryLoader.java:61:
public static void loadAndInitSync() throws ProcessInitException {
On 2013/01/08 21:20:45, joth wrote:
> could you  just delete this method? it's finally not called anywhere any more

Done.

https://codereview.chromium.org/11567061/diff/28001/content/public/common/res...
File content/public/common/result_codes.h (right):

https://codereview.chromium.org/11567061/diff/28001/content/public/common/res...
content/public/common/result_codes.h:12: // c) the exception value that causes a
process to terminate.
Our case belongs to 'c', isn't it?

On 2013/01/08 21:20:45, joth wrote:
> this comment will need updating if we do add this error in here (but see other
> comments)

[michaelbai, 2013-01-09 00:15:05]

I meant the alternative way to handle the error code in
LibraryLoadedOnMainThread.

[joth, 2013-01-09 02:51:24]

lgtm

https://codereview.chromium.org/11567061/diff/28001/android_webview/java/src/...
File android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java
(right):

https://codereview.chromium.org/11567061/diff/28001/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java:55:
throw new Error("Cannot initialize WebView", e);
On 2013/01/09 00:14:00, michaelbai wrote:
> Used RuntimeException.
> 
OK. Was removing  the message intentional? That should work ok with
RuntimeException too. (and seems sufficiently useful to include)



> 
> On 2013/01/08 21:20:45, joth wrote:
> > Unfortunately ThreadUtils.runOnUiThreadBlocking does not propogate Errors
back
> > to the caller, only exceptions.
> > Coud you update ThreadUtils.runOnUiThreadBlocking to catch Throwable instead
> of
> > Exception? (and pass it back through) or just create a RuntimeException here
> > (maybe more reasonable).
> > 
> > 
>

https://codereview.chromium.org/11567061/diff/28001/content/public/common/res...
File content/public/common/result_codes.h (right):

https://codereview.chromium.org/11567061/diff/28001/content/public/common/res...
content/public/common/result_codes.h:12: // c) the exception value that causes a
process to terminate.
On 2013/01/09 00:14:00, michaelbai wrote:
> Our case belongs to 'c', isn't it?
> 

fair enough. (c) was sufficiently general it didn't mean much to me so my eyes
skimmed past it.

Also, conceivably, the webview user can catch and ignore this error, so in that
respect it's an init failure not a process "termination" but yes this is locally
consistent with the terminology we use in chromium content AndroidProcess  etc
so lets go with it like this.

[michaelbai, 2013-01-09 21:04:42]

https://codereview.chromium.org/11567061/diff/28001/android_webview/java/src/...
File android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java
(right):

https://codereview.chromium.org/11567061/diff/28001/android_webview/java/src/...
android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java:55:
throw new Error("Cannot initialize WebView", e);
I were thinking it might be a redundant message, I added it back since you like
it.


On 2013/01/09 02:51:24, joth wrote:
> On 2013/01/09 00:14:00, michaelbai wrote:
> > Used RuntimeException.
> > 
> OK. Was removing  the message intentional? That should work ok with
> RuntimeException too. (and seems sufficiently useful to include)
> 
> 
> 
> > 
> > On 2013/01/08 21:20:45, joth wrote:
> > > Unfortunately ThreadUtils.runOnUiThreadBlocking does not propogate Errors
> back
> > > to the caller, only exceptions.
> > > Coud you update ThreadUtils.runOnUiThreadBlocking to catch Throwable
instead
> > of
> > > Exception? (and pass it back through) or just create a RuntimeException
here
> > > (maybe more reasonable).
> > > 
> > > 
> > 
>

[Ted C, 2013-01-09 23:26:27]

lgtm

[Yaron, 2013-01-10 01:43:20]

lgtm

https://codereview.chromium.org/11567061/diff/29016/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/browser/ContentView.java
(right):

https://codereview.chromium.org/11567061/diff/29016/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ContentView.java:75:
public static boolean enableMultiProcess(Context context, int
maxRendererProcesses)
This can be a separate change but I think we should remove these functsion from
ContentView/ContentViewCore. Given design changes, I don't think it makes sense
to have them here and they're just ultimately forwarding to
AndroidBrowserProcess

[joth, 2013-01-10 03:12:45]

On 9 January 2013 17:43, <yfriedman@chromium.org> wrote:

> lgtm
>
>
>
>
> https://codereview.chromium.**org/11567061/diff/29016/**
> content/public/android/java/**src/org/chromium/content/**
>
browser/ContentView.java<https://codereview.chromium.org/11567061/diff/29016/content/public/android/java/src/org/chromium/content/browser/ContentView.java>
> File
> content/public/android/java/**src/org/chromium/content/**
> browser/ContentView.java
> (right):
>
> https://codereview.chromium.**org/11567061/diff/29016/**
> content/public/android/java/**src/org/chromium/content/**
>
browser/ContentView.java#**newcode75<https://codereview.chromium.org/11567061/diff/29016/content/public/android/java/src/org/chromium/content/browser/ContentView.java#newcode75>
> content/public/android/java/**src/org/chromium/content/**
> browser/ContentView.java:75:
> public static boolean enableMultiProcess(Context context, int
> maxRendererProcesses)
> This can be a separate change but I think we should remove these
> functsion from ContentView/ContentViewCore. Given design changes, I
> don't think it makes sense to have them here and they're just ultimately
> forwarding to AndroidBrowserProcess
>
> +1


>
https://codereview.chromium.**org/11567061/<https://codereview.chromium.org/1...
>

[michaelbai, 2013-01-10 17:46:04]

https://codereview.chromium.org/11567061/diff/29016/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/browser/ContentView.java
(right):

https://codereview.chromium.org/11567061/diff/29016/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ContentView.java:75:
public static boolean enableMultiProcess(Context context, int
maxRendererProcesses)
I will make another CL to remove it.


On 2013/01/10 01:43:20, Yaron wrote:
> This can be a separate change but I think we should remove these functsion
from
> ContentView/ContentViewCore. Given design changes, I don't think it makes
sense
> to have them here and they're just ultimately forwarding to
> AndroidBrowserProcess

[commit-bot: I haz the power, 2013-01-10 17:46:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/michaelbai@chromium.org/11567061/29016

[commit-bot: I haz the power, 2013-01-10 17:46:48]

Presubmit check for 11567061-29016 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/app

Presubmit checks took 1.8s to calculate.

[michaelbai, 2013-01-10 17:55:07]

Hi cpu, please help to review the change of
chrome/app/android/chrome_main_delegate_android.cc, the removed comments and
code were not true any more, the default behavior will not be triggered not
matter what the return value is, we handle the return value in Java side.

[cpu_(ooo_6.6-7.5), 2013-01-11 19:15:32]

lgtm

[commit-bot: I haz the power, 2013-01-11 19:27:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/michaelbai@chromium.org/11567061/29016

[commit-bot: I haz the power, 2013-01-12 13:26:48]

Retried try job too often on win7_aura for step(s) browser_tests

[commit-bot: I haz the power, 2013-01-14 19:40:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/michaelbai@chromium.org/11567061/55001

[commit-bot: I haz the power, 2013-01-15 21:35:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/michaelbai@chromium.org/11567061/55001

[commit-bot: I haz the power, 2013-01-15 21:43:21]

Change committed as 176975