Renamed Sandboxed process to Child process

Since some child processes, such as PPAPI_BROKER plugin, might not run in sandbox, Sandboxed process is renamed to Child process

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

[kjyoun, 2013-03-06 00:18:00]

Can you review this change?

[palmer, 2013-03-06 00:36:26]

https://codereview.chromium.org/12321131/diff/1034/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/app/SandboxedProcessService.java
(right):

https://codereview.chromium.org/12321131/diff/1034/content/public/android/jav...
content/public/android/java/src/org/chromium/content/app/SandboxedProcessService.java:10:
public class SandboxedProcessService extends ChildProcessService {
Is this class supposed to be empty?

[kjyoun, 2013-03-06 07:34:08]

https://codereview.chromium.org/12321131/diff/1034/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/app/SandboxedProcessService.java
(right):

https://codereview.chromium.org/12321131/diff/1034/content/public/android/jav...
content/public/android/java/src/org/chromium/content/app/SandboxedProcessService.java:10:
public class SandboxedProcessService extends ChildProcessService {
On 2013/03/06 00:36:26, Chris P. wrote:
> Is this class supposed to be empty?
Yes, though all logic of SandboxedProcessService moved to ChildProcessService,
this Class remains as a stub, since it is referenced to get base name of
SandboxedProcessService# in ChildProcessConnection.

    private Intent createServiceBindIntent() {
        Intent intent = new Intent();
        String n =
org.chromium.content.app.SandboxedProcessService.class.getName();
        intent.setClassName(mContext, n + mServiceNumber);
        intent.setPackage(mContext.getPackageName());
        return intent;
    }

[joth, 2013-03-06 17:48:03]

https://codereview.chromium.org/12321131/diff/1034/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java
(right):

https://codereview.chromium.org/12321131/diff/1034/content/public/android/jav...
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java:36:
*     <service android:name="org.chromium.content.app.SandboxedProcessServiceX"
Why aren't SandboxedProcessService0 ... SandboxedProcessService5 classes also
renamed? This documentation seems confusing now - if a ChildProcessLauncher can
only connect to SandboxedProcessService0..5 why not leave it called
SandboxedProcessLauncher?

https://codereview.chromium.org/12321131/diff/1034/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/app/SandboxedProcessService.java
(right):

https://codereview.chromium.org/12321131/diff/1034/content/public/android/jav...
content/public/android/java/src/org/chromium/content/app/SandboxedProcessService.java:10:
public class SandboxedProcessService extends ChildProcessService {
On 2013/03/06 07:34:08, kjyoun wrote:
> On 2013/03/06 00:36:26, Chris P. wrote:
> > Is this class supposed to be empty?
> Yes, though all logic of SandboxedProcessService moved to ChildProcessService,
> this Class remains as a stub, since it is referenced to get base name of
> SandboxedProcessService# in ChildProcessConnection.
> 
>     private Intent createServiceBindIntent() {
>         Intent intent = new Intent();
>         String n =
> org.chromium.content.app.SandboxedProcessService.class.getName();
>         intent.setClassName(mContext, n + mServiceNumber);
>         intent.setPackage(mContext.getPackageName());
>         return intent;
>     }
> 
> 

If it's only used to get the base-name of the sub classes, you're probably
better off replacing that by a string constant.

[palmer, 2013-03-06 18:52:06]

> Why aren't SandboxedProcessService0 ... SandboxedProcessService5 classes also
> renamed?

Those are declared in the AndroidManifest.xml, where they declare isolated
Service processes — those processes actually are sandboxed. So that's the right
name for those processes. But not all children are sandboxed.

> If it's only used to get the base-name of the sub classes, you're probably
> better off replacing that by a string constant.

Agreed.

[joth, 2013-03-06 19:23:56]

On 6 March 2013 10:52, <palmer@chromium.org> wrote:

> Why aren't SandboxedProcessService0 ... SandboxedProcessService5 classes
>> also
>> renamed?
>>
>
> Those are declared in the AndroidManifest.xml, where they declare isolated
> Service processes — those processes actually are sandboxed. So that's the
> right
> name for those processes. But not all children are sandboxed.
>
>
OK, but to my second point - ChildProcessService is only documenting how to
setup a sandboxed service connection. How is it used for a non-sandboxed
service?




>
>  If it's only used to get the base-name of the sub classes, you're probably
>> better off replacing that by a string constant.
>>
>
> Agreed.
>
>
https://codereview.chromium.**org/12321131/<https://codereview.chromium.org/1...
>

[kjyoun, 2013-03-07 09:27:39]

As title mentioned, this CL is to rename some of SandboxedProcess to
ChildProcess, depending on context.
Design for how to launch NonSandboxedProcess is depicted on http://goo.gl/7nUpp
(Not to obscure central idea of non-sandbox process, I upload renaming as a
separate CL).

On 2013/03/06 19:23:56, joth wrote:
> On 6 March 2013 10:52, <mailto:palmer@chromium.org> wrote:
> 
> > Why aren't SandboxedProcessService0 ... SandboxedProcessService5 classes
> >> also
> >> renamed?
> >>
> >
> > Those are declared in the AndroidManifest.xml, where they declare isolated
> > Service processes — those processes actually are sandboxed. So that's the
> > right
> > name for those processes. But not all children are sandboxed.
> >
> >
> OK, but to my second point - ChildProcessService is only documenting how to
> setup a sandboxed service connection. How is it used for a non-sandboxed
> service?
> 
> 
> 
> 
> >
> >  If it's only used to get the base-name of the sub classes, you're probably
> >> better off replacing that by a string constant.
> >>
> >
> > Agreed.
> >
> >
>
https://codereview.chromium.**org/12321131/%3Chttps://codereview.chromium.org...>
> >

[kjyoun, 2013-03-07 09:52:08]

https://codereview.chromium.org/12321131/diff/1034/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/app/SandboxedProcessService.java
(right):

https://codereview.chromium.org/12321131/diff/1034/content/public/android/jav...
content/public/android/java/src/org/chromium/content/app/SandboxedProcessService.java:10:
public class SandboxedProcessService extends ChildProcessService {
Agree, and will do
On 2013/03/06 17:48:03, joth wrote:
> On 2013/03/06 07:34:08, kjyoun wrote:
> > On 2013/03/06 00:36:26, Chris P. wrote:
> > > Is this class supposed to be empty?
> > Yes, though all logic of SandboxedProcessService moved to
ChildProcessService,
> > this Class remains as a stub, since it is referenced to get base name of
> > SandboxedProcessService# in ChildProcessConnection.
> > 
> >     private Intent createServiceBindIntent() {
> >         Intent intent = new Intent();
> >         String n =
> > org.chromium.content.app.SandboxedProcessService.class.getName();
> >         intent.setClassName(mContext, n + mServiceNumber);
> >         intent.setPackage(mContext.getPackageName());
> >         return intent;
> >     }
> > 
> > 
> 
> If it's only used to get the base-name of the sub classes, you're probably
> better off replacing that by a string constant.

[kjyoun, 2013-03-07 11:40:00]

https://codereview.chromium.org/12321131/diff/1034/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java
(right):

https://codereview.chromium.org/12321131/diff/1034/content/public/android/jav...
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java:36:
*     <service android:name="org.chromium.content.app.SandboxedProcessServiceX"
SandboxedProcessService0..5 is not renamed, since they are used, inherited from
ChildProcessService. They are ChildProcess running inside of Sandbox (Isolated
process in Android)
On 2013/03/06 17:48:03, joth wrote:
> Why aren't SandboxedProcessService0 ... SandboxedProcessService5 classes also
> renamed? This documentation seems confusing now - if a ChildProcessLauncher
can
> only connect to SandboxedProcessService0..5 why not leave it called
> SandboxedProcessLauncher?

[joth, 2013-03-07 19:58:54]

lgtm

https://codereview.chromium.org/12321131/diff/1034/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java
(right):

https://codereview.chromium.org/12321131/diff/1034/content/public/android/jav...
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java:36:
*     <service android:name="org.chromium.content.app.SandboxedProcessServiceX"
On 2013/03/07 11:40:00, kjyoun wrote:
> SandboxedProcessService0..5 is not renamed, since they are used, inherited
from
> ChildProcessService. They are ChildProcess running inside of Sandbox (Isolated
> process in Android)
> On 2013/03/06 17:48:03, joth wrote:
> > Why aren't SandboxedProcessService0 ... SandboxedProcessService5 classes
also
> > renamed? This documentation seems confusing now - if a ChildProcessLauncher
> can
> > only connect to SandboxedProcessService0..5 why not leave it called
> > SandboxedProcessLauncher?
> 

OK I got that now. Makes sense to handle this in separate CLs (rename vs add new
functionality) so how about putting a TODO here to update the docs when
non-sandboxed service support is added?

[Yaron, 2013-03-08 03:10:30]

lgtm

I pointed out several places where comments need updating but this seems
functionally correct

https://codereview.chromium.org/12321131/diff/13001/content/app/android/child...
File content/app/android/child_process_service.cc (right):

https://codereview.chromium.org/12321131/diff/13001/content/app/android/child...
content/app/android/child_process_service.cc:81: void
QuitSandboxMainThreadMessageLoop() {
QuitMainThreadMessageLoop

https://codereview.chromium.org/12321131/diff/13001/content/app/android/child...
content/app/android/child_process_service.cc:117: void
ShutdownSandboxMainThread(JNIEnv* env, jobject obj) {
ShutdownMainThread

https://codereview.chromium.org/12321131/diff/13001/content/app/android/child...
File content/app/android/child_process_service.h (right):

https://codereview.chromium.org/12321131/diff/13001/content/app/android/child...
content/app/android/child_process_service.h:5: #ifndef
CONTENT_APP_ANDROID_SANDBOXED_PROCESS_SERVICE_H_
Update header guard

https://codereview.chromium.org/12321131/diff/13001/content/browser/android/b...
File content/browser/android/browser_jni_registrar.cc (right):

https://codereview.chromium.org/12321131/diff/13001/content/browser/android/b...
content/browser/android/browser_jni_registrar.cc:48: { "ChildProcessLauncher",
content::RegisterChildProcessLauncher },
Nit: alpha sort

https://codereview.chromium.org/12321131/diff/13001/content/browser/android/c...
File content/browser/android/child_process_launcher.h (right):

https://codereview.chromium.org/12321131/diff/13001/content/browser/android/c...
content/browser/android/child_process_launcher.h:5: #ifndef
CONTENT_BROWSER_ANDROID_SANDBOXED_PROCESS_LAUNCHER_H_
Update

https://codereview.chromium.org/12321131/diff/13001/content/browser/android/c...
content/browser/android/child_process_launcher.h:19: // Starts a process as a
sandboxed process spawned by the Android
update comment

https://codereview.chromium.org/12321131/diff/13001/content/browser/android/c...
content/browser/android/child_process_launcher.h:28: // Stops a sandboxed
process based on the handle returned form
update comment

https://codereview.chromium.org/12321131/diff/13001/content/browser/android/c...
content/browser/android/child_process_launcher.h:32: // Registers JNI methods,
this must be called before any other methods in this
Can you remove this comment while you're here? It's unnecessary

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java
(right):

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java:30:
* This is the base class for sandboxed services; the SandboxedProcessService0,
1.. etc
Update comment

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java:46:
private Thread mSandboxMainThread;
rename

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/browser/AndroidBrowserProcess.java
(right):

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/AndroidBrowserProcess.java:53:
//   6: The maximum number of sandboxed processes we have available
update comment

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
(right):

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:65:
Log.w(TAG, "Ran out of sandboxed services.");
Update comment

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:80:
assert libraryName != null : "Attempting to launch a sandbox process without
first "
Update comment

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:124:
* Returns the sandboxed process service interface for the given pid. This may be
called on
Update comment

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:152:
* Spawns and connects to a sandboxed process. May be called on any thread. It
will not
Update comment

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:213:
* Terminates a sandboxed process. This may be called from any thread.
Update comment

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:231:
* Bind a sandboxed process as a high priority process so that it has the same
Update comment

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/common/IChildProcessCallback.aidl
(right):

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/common/IChildProcessCallback.aidl:11:
// Conduit to pass a Surface from the sandboxed renderer to the plugin.
Update comment

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/common/IChildProcessService.aidl
(right):

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/common/IChildProcessService.aidl:13:
// Sets up the initial IPC channel and returns the pid of the sandboxed process.
Update comment

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
File content/public/android/java/src/org/chromium/content/common/common.aidl
(right):

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/common/common.aidl:5: //
This file is needed to compile ISandBoxedProcessCallback.aidl and
Update comment

[kjyoun, 2013-03-11 14:17:43]

https://codereview.chromium.org/12321131/diff/13001/content/app/android/child...
File content/app/android/child_process_service.cc (right):

https://codereview.chromium.org/12321131/diff/13001/content/app/android/child...
content/app/android/child_process_service.cc:81: void
QuitSandboxMainThreadMessageLoop() {
On 2013/03/08 03:10:31, Yaron wrote:
> QuitMainThreadMessageLoop

Done.

https://codereview.chromium.org/12321131/diff/13001/content/app/android/child...
content/app/android/child_process_service.cc:117: void
ShutdownSandboxMainThread(JNIEnv* env, jobject obj) {
On 2013/03/08 03:10:31, Yaron wrote:
> ShutdownMainThread

Done.

https://codereview.chromium.org/12321131/diff/13001/content/app/android/child...
File content/app/android/child_process_service.h (right):

https://codereview.chromium.org/12321131/diff/13001/content/app/android/child...
content/app/android/child_process_service.h:5: #ifndef
CONTENT_APP_ANDROID_SANDBOXED_PROCESS_SERVICE_H_
On 2013/03/08 03:10:31, Yaron wrote:
> Update header guard

Done.

https://codereview.chromium.org/12321131/diff/13001/content/browser/android/b...
File content/browser/android/browser_jni_registrar.cc (right):

https://codereview.chromium.org/12321131/diff/13001/content/browser/android/b...
content/browser/android/browser_jni_registrar.cc:48: { "ChildProcessLauncher",
content::RegisterChildProcessLauncher },
On 2013/03/08 03:10:31, Yaron wrote:
> Nit: alpha sort

Done.

https://codereview.chromium.org/12321131/diff/13001/content/browser/android/c...
File content/browser/android/child_process_launcher.h (right):

https://codereview.chromium.org/12321131/diff/13001/content/browser/android/c...
content/browser/android/child_process_launcher.h:5: #ifndef
CONTENT_BROWSER_ANDROID_SANDBOXED_PROCESS_LAUNCHER_H_
On 2013/03/08 03:10:31, Yaron wrote:
> Update

Done.

https://codereview.chromium.org/12321131/diff/13001/content/browser/android/c...
content/browser/android/child_process_launcher.h:19: // Starts a process as a
sandboxed process spawned by the Android
On 2013/03/08 03:10:31, Yaron wrote:
> update comment

Done.

https://codereview.chromium.org/12321131/diff/13001/content/browser/android/c...
content/browser/android/child_process_launcher.h:28: // Stops a sandboxed
process based on the handle returned form
On 2013/03/08 03:10:31, Yaron wrote:
> update comment

Done.

https://codereview.chromium.org/12321131/diff/13001/content/browser/android/c...
content/browser/android/child_process_launcher.h:32: // Registers JNI methods,
this must be called before any other methods in this
On 2013/03/08 03:10:31, Yaron wrote:
> Can you remove this comment while you're here? It's unnecessary

Done.

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java
(right):

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java:30:
* This is the base class for sandboxed services; the SandboxedProcessService0,
1.. etc
On 2013/03/08 03:10:31, Yaron wrote:
> Update comment

Done.

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/app/ChildProcessService.java:46:
private Thread mSandboxMainThread;
On 2013/03/08 03:10:31, Yaron wrote:
> rename

Done. renamed to mMainThread

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/browser/AndroidBrowserProcess.java
(right):

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/AndroidBrowserProcess.java:53:
//   6: The maximum number of sandboxed processes we have available
On 2013/03/08 03:10:31, Yaron wrote:
> update comment

Done.

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
(right):

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:80:
assert libraryName != null : "Attempting to launch a sandbox process without
first "
On 2013/03/08 03:10:31, Yaron wrote:
> Update comment

Done.

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:124:
* Returns the sandboxed process service interface for the given pid. This may be
called on
On 2013/03/08 03:10:31, Yaron wrote:
> Update comment

Done.

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:152:
* Spawns and connects to a sandboxed process. May be called on any thread. It
will not
On 2013/03/08 03:10:31, Yaron wrote:
> Update comment

Done.

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/common/IChildProcessCallback.aidl
(right):

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/common/IChildProcessCallback.aidl:11:
// Conduit to pass a Surface from the sandboxed renderer to the plugin.
"sandboxed render" looks correct, since render process runs in sandbox
On 2013/03/08 03:10:31, Yaron wrote:
> Update comment

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/common/IChildProcessService.aidl
(right):

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/common/IChildProcessService.aidl:13:
// Sets up the initial IPC channel and returns the pid of the sandboxed process.
On 2013/03/08 03:10:31, Yaron wrote:
> Update comment

Done.

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
File content/public/android/java/src/org/chromium/content/common/common.aidl
(right):

https://codereview.chromium.org/12321131/diff/13001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/common/common.aidl:5: //
This file is needed to compile ISandBoxedProcessCallback.aidl and
On 2013/03/08 03:10:31, Yaron wrote:
> Update comment

Done.

[Yaron, 2013-03-11 20:09:19]

lgtm

Note that there are probably findbugs changes that you need. I think you can
first build all and then run
build/android/findbugs_diff.py -r to rebaseline with your file moves

[Yaron, 2013-03-11 23:54:31]

On 2013/03/11 20:09:19, Yaron wrote:
> lgtm
> 
> Note that there are probably findbugs changes that you need. I think you can
> first build all and then run
> build/android/findbugs_diff.py -r to rebaseline with your file moves

Unchecked commit box. IF you look at the findbugs file, it's clearly not
correct. Please ensure you've built all targets first and then update the
whitelist.

[kjyoun, 2013-03-12 00:20:49]

Running bu
Running below script again to ensure that all targets are build properly.

. ./build/android/envsetup.sh
android_gyp
ninja -C out/Release -j 25
build/android/findbugs_diff.py -r


On 2013/03/11 23:54:31, Yaron wrote:
> On 2013/03/11 20:09:19, Yaron wrote:
> > lgtm
> > 
> > Note that there are probably findbugs changes that you need. I think you can
> > first build all and then run
> > build/android/findbugs_diff.py -r to rebaseline with your file moves
> 
> Unchecked commit box. IF you look at the findbugs file, it's clearly not
> correct. Please ensure you've built all targets first and then update the
> whitelist.

[Yaron, 2013-03-12 00:32:02]

I think the problem is that findbugs probably looks in your debug folder by
default.  Pass --release-build might correct the issue.

[kjyoun, 2013-03-12 01:23:40]

On 2013/03/12 00:32:02, Yaron wrote:
> I think the problem is that findbugs probably looks in your debug folder by
> default.  Pass --release-build might correct the issue.

It worked.
I uploaded new CL with updated findbugs_known_bugs.txt and initiated commit

[commit-bot: I haz the power, 2013-03-12 01:24:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kjyoun@google.com/12321131/41002

[commit-bot: I haz the power, 2013-03-12 01:24:39]

Presubmit check for 12321131-41002 failed and returned exit status 1.

INFO:root:Found 31 file(s).

INFO:PRESUBMIT:Skipping pylint: no matching changes.
Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/build/android/PRESUBMIT.py
Running
/b/commit-queue/workdir/chromium/build/android/buildbot/tests/bb_run_bot_test.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    content/content_app.gypi
    content/content_browser.gypi
    content/content.gyp
    content/browser/child_process_launcher.cc

Following files should only contain deletions.
  build/android/findbugs_filter/findbugs_known_bugs.txt

Presubmit checks took 4.5s to calculate.

[kjyoun, 2013-03-12 01:30:27]

Avi/Brett/Scott

Can you review this CL?

[kjyoun, 2013-03-13 05:07:45]

Gentle reminder.

[Avi (use Gerrit), 2013-03-13 14:25:16]

stampity stamp lgtm

[commit-bot: I haz the power, 2013-03-13 20:53:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kjyoun@google.com/12321131/41002

[commit-bot: I haz the power, 2013-03-13 20:53:40]

Presubmit check for 12321131-41002 failed and returned exit status 1.

INFO:root:Found 31 file(s).

INFO:PRESUBMIT:Skipping pylint: no matching changes.
Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/build/android/PRESUBMIT.py
Running
/b/commit-queue/workdir/chromium/build/android/buildbot/tests/bb_run_bot_test.py

** Presubmit ERRORS **
Following files should only contain deletions.
  build/android/findbugs_filter/findbugs_known_bugs.txt

Presubmit checks took 7.7s to calculate.

[kjyoun, 2013-03-13 21:00:46]

Can you let me know how to handle this error?

findbugs_known_bugs.txt allows only deletion,
my changes has some renames, which seems to make error.

 M D SF: Switch statement found in
org.chromium.content.browser.third_party.GestureDetector.onTouchEvent(MotionEvent)
where default case is missing At GestureDetector.java -M D ST: Write to
static field org.chromium.content.app.SandboxedProcessService.sContext from
instance method org.chromium.content.app.SandboxedProcessService.onCreate()
At SandboxedProcessService.java +M D ST: Write to static field
org.chromium.content.app.ChildProcessService.sContext from instance method
org.chromium.content.app.ChildProcessService.onCreate() At
ChildProcessService.java M D ST: Write to static field
org.chromium.content.browser.ContentSettings.sAppCachePathIsSet from
instance method
org.chromium.content.browser.ContentSettings.setAppCachePath(String) At
ContentSettings.java





On Thu, Mar 14, 2013 at 5:53 AM, <commit-bot@chromium.org> wrote:

> Presubmit check for 12321131-41002 failed and returned exit status 1.
>
> INFO:root:Found 31 file(s).
>
> INFO:PRESUBMIT:Skipping pylint: no matching changes.
> Running presubmit commit checks ...
> Running /b/commit-queue/workdir/**chromium/PRESUBMIT.py
> Running /b/commit-queue/workdir/**chromium/build/android/**PRESUBMIT.py
> Running
> /b/commit-queue/workdir/**chromium/build/android/**
> buildbot/tests/bb_run_bot_**test.py
>
> ** Presubmit ERRORS **
> Following files should only contain deletions.
>   build/android/findbugs_filter/**findbugs_known_bugs.txt
>
> Presubmit checks took 7.7s to calculate.
>
>
>
>
https://chromiumcodereview.**appspot.com/12321131/<https://chromiumcodereview...
>



-- 

KJ Youn/연규정 | Technical Solution Engineer |  kjyoun@google.com |
+82-10-8162-9397
Too brief? Here's why! http://emailcharter.org

[Yaron, 2013-03-13 21:00:48]

https://chromiumcodereview.appspot.com/12321131/diff/41002/build/android/find...
File build/android/findbugs_filter/findbugs_known_bugs.txt (right):

https://chromiumcodereview.appspot.com/12321131/diff/41002/build/android/find...
build/android/findbugs_filter/findbugs_known_bugs.txt:69: M D REC: Exception is
caught when Exception is not thrown in
org.chromium.content.browser.TextInputTypeTest.isInputVisible(ContentView)  At
TextInputTypeTest.java
Looks like you need to remove this one?

[joth, 2013-03-13 21:04:54]

On 13 March 2013 13:59, KJ Youn <kjyoun@google.com> wrote:

> Can you let me know how to handle this error?
>
> findbugs_known_bugs.txt allows only deletion,
> my changes has some renames, which seems to make error.
>
>  M D SF: Switch statement found in
>
org.chromium.content.browser.third_party.GestureDetector.onTouchEvent(MotionEvent)
> where default case is missing At GestureDetector.java -M D ST: Write to
> static field org.chromium.content.app.SandboxedProcessService.sContext from
> instance method org.chromium.content.app.SandboxedProcessService.onCreate()
> At SandboxedProcessService.java +M D ST: Write to static field
> org.chromium.content.app.ChildProcessService.sContext from instance method
> org.chromium.content.app.ChildProcessService.onCreate() At
> ChildProcessService.java M D ST: Write to static field
> org.chromium.content.browser.ContentSettings.sAppCachePathIsSet from
> instance method
> org.chromium.content.browser.ContentSettings.setAppCachePath(String) At
> ContentSettings.java
>
>
>
>
My guess would be to wrap it in a static setContext() method.
http://stackoverflow.com/questions/4457042/best-practice-for-updating-writing...
to agree.


>
>
> On Thu, Mar 14, 2013 at 5:53 AM, <commit-bot@chromium.org> wrote:
>
>> Presubmit check for 12321131-41002 failed and returned exit status 1.
>>
>> INFO:root:Found 31 file(s).
>>
>> INFO:PRESUBMIT:Skipping pylint: no matching changes.
>> Running presubmit commit checks ...
>> Running /b/commit-queue/workdir/**chromium/PRESUBMIT.py
>> Running /b/commit-queue/workdir/**chromium/build/android/**PRESUBMIT.py
>> Running
>> /b/commit-queue/workdir/**chromium/build/android/**
>> buildbot/tests/bb_run_bot_**test.py
>>
>> ** Presubmit ERRORS **
>> Following files should only contain deletions.
>>   build/android/findbugs_filter/**findbugs_known_bugs.txt
>>
>> Presubmit checks took 7.7s to calculate.
>>
>>
>>
>>
https://chromiumcodereview.**appspot.com/12321131/<https://chromiumcodereview...
>>
>
>
>
> --
>
> KJ Youn/연규정 | Technical Solution Engineer |  kjyoun@google.com |
> +82-10-8162-9397
> Too brief? Here's why! http://emailcharter.org
>

[kjyoun, 2013-03-13 21:06:17]

Can you let me know how to handle this error?

findbugs_known_bugs.txt allows only deletion,
my changes has some renames, which seems to make error.

 M D SF: Switch statement found in
org.chromium.content.browser.third_party.GestureDetector.onTouchEvent(MotionEvent)
where default case is missing At GestureDetector.java -M D ST: Write to
static field org.chromium.content.app.SandboxedProcessService.sContext from
instance method org.chromium.content.app.SandboxedProcessService.onCreate()
At SandboxedProcessService.java +M D ST: Write to static field
org.chromium.content.app.ChildProcessService.sContext from instance method
org.chromium.content.app.ChildProcessService.onCreate() At
ChildProcessService.java M D ST: Write to static field
org.chromium.content.browser.ContentSettings.sAppCachePathIsSet from
instance method
org.chromium.content.browser.ContentSettings.setAppCachePath(String) At
ContentSettings.java





On Thu, Mar 14, 2013 at 5:53 AM, <commit-bot@chromium.org> wrote:

> Presubmit check for 12321131-41002 failed and returned exit status 1.
>
> INFO:root:Found 31 file(s).
>
> INFO:PRESUBMIT:Skipping pylint: no matching changes.
> Running presubmit commit checks ...
> Running /b/commit-queue/workdir/**chromium/PRESUBMIT.py
> Running /b/commit-queue/workdir/**chromium/build/android/**PRESUBMIT.py
> Running
> /b/commit-queue/workdir/**chromium/build/android/**
> buildbot/tests/bb_run_bot_**test.py
>
> ** Presubmit ERRORS **
> Following files should only contain deletions.
>   build/android/findbugs_filter/**findbugs_known_bugs.txt
>
> Presubmit checks took 7.7s to calculate.
>
>
>
>
https://chromiumcodereview.**appspot.com/12321131/<https://chromiumcodereview...
>



-- 

KJ Youn/연규정 | Technical Solution Engineer |  kjyoun@google.com |
+82-10-8162-9397
Too brief? Here's why! http://emailcharter.org

[kjyoun, 2013-03-14 01:53:52]

https://chromiumcodereview.appspot.com/12321131/diff/41002/build/android/find...
File build/android/findbugs_filter/findbugs_known_bugs.txt (right):

https://chromiumcodereview.appspot.com/12321131/diff/41002/build/android/find...
build/android/findbugs_filter/findbugs_known_bugs.txt:69: M D REC: Exception is
caught when Exception is not thrown in
org.chromium.content.browser.TextInputTypeTest.isInputVisible(ContentView)  At
TextInputTypeTest.java

On 2013/03/13 21:00:48, Yaron wrote:
> Looks like you need to remove this one?
Unfortunately, removal of this line did pass pre-submit check.

[kjyoun, 2013-03-14 02:17:34]

On 2013/03/14 01:53:52, kjyoun wrote:
>
https://chromiumcodereview.appspot.com/12321131/diff/41002/build/android/find...
> File build/android/findbugs_filter/findbugs_known_bugs.txt (right):
> 
>
https://chromiumcodereview.appspot.com/12321131/diff/41002/build/android/find...
> build/android/findbugs_filter/findbugs_known_bugs.txt:69: M D REC: Exception
is
> caught when Exception is not thrown in
> org.chromium.content.browser.TextInputTypeTest.isInputVisible(ContentView)  At
> TextInputTypeTest.java
> 
> On 2013/03/13 21:00:48, Yaron wrote:
> > Looks like you need to remove this one?
> Unfortunately, removal of this line did pass pre-submit check.

Not to add bugs to findbugs_known_bugs.txt, it looks like that I should fix
known bugs in SandboxedProcess. Otherwise, they would be added as new known bugs
in ChildProcess.

Is there any workaround for this?
The CL is not to do add any logical change beyond renaming

[kjyoun, 2013-03-14 06:59:29]

This issue looks very tricky.
In addition to that find_known_bugs.txt does not allow update from renaming, 
current find_known_bugs.txt is not consistent with latest code.
If I generate dbug_known_bugs.txt from latest code (w/o my CL),
I cannot pass pre-commit check.

It might not be a valid option to resolve those newly-added known bugs in this
CL.

On 2013/03/14 02:17:34, kjyoun wrote:
> On 2013/03/14 01:53:52, kjyoun wrote:
> >
>
https://chromiumcodereview.appspot.com/12321131/diff/41002/build/android/find...
> > File build/android/findbugs_filter/findbugs_known_bugs.txt (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/12321131/diff/41002/build/android/find...
> > build/android/findbugs_filter/findbugs_known_bugs.txt:69: M D REC: Exception
> is
> > caught when Exception is not thrown in
> > org.chromium.content.browser.TextInputTypeTest.isInputVisible(ContentView) 
At
> > TextInputTypeTest.java
> > 
> > On 2013/03/13 21:00:48, Yaron wrote:
> > > Looks like you need to remove this one?
> > Unfortunately, removal of this line did pass pre-submit check.
> 
> Not to add bugs to findbugs_known_bugs.txt, it looks like that I should fix
> known bugs in SandboxedProcess. Otherwise, they would be added as new known
bugs
> in ChildProcess.
> 
> Is there any workaround for this?
> The CL is not to do add any logical change beyond renaming

[Yaron, 2013-03-14 19:58:56]

On 2013/03/14 06:59:29, kjyoun wrote:
> This issue looks very tricky.
> In addition to that find_known_bugs.txt does not allow update from renaming, 
> current find_known_bugs.txt is not consistent with latest code.
> If I generate dbug_known_bugs.txt from latest code (w/o my CL),
> I cannot pass pre-commit check.
> 
> It might not be a valid option to resolve those newly-added known bugs in this
> CL.
> 
> On 2013/03/14 02:17:34, kjyoun wrote:
> > On 2013/03/14 01:53:52, kjyoun wrote:
> > >
> >
>
https://chromiumcodereview.appspot.com/12321131/diff/41002/build/android/find...
> > > File build/android/findbugs_filter/findbugs_known_bugs.txt (right):
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/12321131/diff/41002/build/android/find...
> > > build/android/findbugs_filter/findbugs_known_bugs.txt:69: M D REC:
Exception
> > is
> > > caught when Exception is not thrown in
> > > org.chromium.content.browser.TextInputTypeTest.isInputVisible(ContentView)

> At
> > > TextInputTypeTest.java
> > > 
> > > On 2013/03/13 21:00:48, Yaron wrote:
> > > > Looks like you need to remove this one?
> > > Unfortunately, removal of this line did pass pre-submit check.
> > 
> > Not to add bugs to findbugs_known_bugs.txt, it looks like that I should fix
> > known bugs in SandboxedProcess. Otherwise, they would be added as new known
> bugs
> > in ChildProcess.
> > 
> > Is there any workaround for this?
> > The CL is not to do add any logical change beyond renaming

I cc'd you on https://codereview.chromium.org/12825011/ which should make the
presubmit by-passable for cases such as this

[kjyoun, 2013-03-14 23:21:17]

Thanks!
Let me try again with updated presubmit hook.


On Fri, Mar 15, 2013 at 4:58 AM, <yfriedman@chromium.org> wrote:

> On 2013/03/14 06:59:29, kjyoun wrote:
>
>> This issue looks very tricky.
>> In addition to that find_known_bugs.txt does not allow update from
>> renaming,
>> current find_known_bugs.txt is not consistent with latest code.
>> If I generate dbug_known_bugs.txt from latest code (w/o my CL),
>> I cannot pass pre-commit check.
>>
>
>  It might not be a valid option to resolve those newly-added known bugs in
>> this
>> CL.
>>
>
>  On 2013/03/14 02:17:34, kjyoun wrote:
>> > On 2013/03/14 01:53:52, kjyoun wrote:
>> > >
>> >
>>
>
> https://chromiumcodereview.**appspot.com/12321131/diff/**
>
41002/build/android/findbugs_**filter/findbugs_known_bugs.txt<https://chromiumcodereview.appspot.com/12321131/diff/41002/build/android/findbugs_filter/findbugs_known_bugs.txt>
>
>> > > File build/android/findbugs_filter/**findbugs_known_bugs.txt (right):
>> > >
>> > >
>> >
>>
>
> https://chromiumcodereview.**appspot.com/12321131/diff/**
>
41002/build/android/findbugs_**filter/findbugs_known_bugs.**txt#newcode69<https://chromiumcodereview.appspot.com/12321131/diff/41002/build/android/findbugs_filter/findbugs_known_bugs.txt#newcode69>
>
>> > > build/android/findbugs_filter/**findbugs_known_bugs.txt:69: M D REC:
>>
> Exception
>
>> > is
>> > > caught when Exception is not thrown in
>> > > org.chromium.content.browser.**TextInputTypeTest.**
>> isInputVisible(ContentView)
>>
>
>  At
>> > > TextInputTypeTest.java
>> > >
>> > > On 2013/03/13 21:00:48, Yaron wrote:
>> > > > Looks like you need to remove this one?
>> > > Unfortunately, removal of this line did pass pre-submit check.
>> >
>> > Not to add bugs to findbugs_known_bugs.txt, it looks like that I should
>> fix
>> > known bugs in SandboxedProcess. Otherwise, they would be added as new
>> known
>> bugs
>> > in ChildProcess.
>> >
>> > Is there any workaround for this?
>> > The CL is not to do add any logical change beyond renaming
>>
>
> I cc'd you on
https://codereview.chromium.**org/12825011/<https://codereview.chromium.org/1...
should make the
> presubmit by-passable for cases such as this
>
>
https://codereview.chromium.**org/12321131/<https://codereview.chromium.org/1...
>



-- 

KJ Youn/연규정 | Technical Solution Engineer |  kjyoun@google.com |
+82-10-8162-9397
Too brief? Here's why! http://emailcharter.org

[commit-bot: I haz the power, 2013-03-15 01:08:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kjyoun@google.com/12321131/41002

[commit-bot: I haz the power, 2013-03-15 01:08:32]

Presubmit check for 12321131-41002 failed and returned exit status 1.

INFO:root:Found 31 file(s).

INFO:PRESUBMIT:Skipping pylint: no matching changes.
Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/build/android/PRESUBMIT.py
Running
/b/commit-queue/workdir/chromium/build/android/buildbot/tests/bb_run_bot_test.py

** Presubmit Warnings **
Following files should only contain deletions.
  build/android/findbugs_filter/findbugs_known_bugs.txt

Presubmit checks took 5.0s to calculate.

[kjyoun, 2013-03-15 02:00:13]

I still met the same error.

Is this because Yaron's change not applied to buildbot yet?
(Let me try a few hours later)

On 2013/03/15 01:08:32, I haz the power (commit-bot) wrote:
> Presubmit check for 12321131-41002 failed and returned exit status 1.
> 
> INFO:root:Found 31 file(s).
> 
> INFO:PRESUBMIT:Skipping pylint: no matching changes.
> Running presubmit commit checks ...
> Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
> Running /b/commit-queue/workdir/chromium/build/android/PRESUBMIT.py
> Running
>
/b/commit-queue/workdir/chromium/build/android/buildbot/tests/bb_run_bot_test.py
> 
> ** Presubmit Warnings **
> Following files should only contain deletions.
>   build/android/findbugs_filter/findbugs_known_bugs.txt
> 
> Presubmit checks took 5.0s to calculate.

[commit-bot: I haz the power, 2013-03-15 04:25:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kjyoun@google.com/12321131/41002

[commit-bot: I haz the power, 2013-03-15 04:26:10]

Presubmit check for 12321131-41002 failed and returned exit status 1.

INFO:root:Found 31 file(s).

INFO:PRESUBMIT:Skipping pylint: no matching changes.
Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/build/android/PRESUBMIT.py
Running
/b/commit-queue/workdir/chromium/build/android/buildbot/tests/bb_run_bot_test.py

** Presubmit Warnings **
Following files should only contain deletions.
  build/android/findbugs_filter/findbugs_known_bugs.txt

Presubmit checks took 4.8s to calculate.

[kjyoun, 2013-03-15 07:06:28]

Still no success
Message was changed from errors to warnings
But, buildbot still failed

[commit-bot: I haz the power, 2013-03-15 14:29:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kjyoun@google.com/12321131/69001

[commit-bot: I haz the power, 2013-03-15 14:30:13]

Presubmit check for 12321131-69001 failed and returned exit status 1.

INFO:root:Found 31 file(s).

INFO:PRESUBMIT:Skipping pylint: no matching changes.
Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/build/android/PRESUBMIT.py
Running
/b/commit-queue/workdir/chromium/build/android/buildbot/tests/bb_run_bot_test.py

** Presubmit Warnings **
Following files should only contain deletions.
  build/android/findbugs_filter/findbugs_known_bugs.txt

Presubmit checks took 5.0s to calculate.

[Yaron, 2013-03-15 16:40:37]

Looks like my warnings change is insufficient. You can either land with dcommit
or we need to fix the findbugs issues

https://chromiumcodereview.appspot.com/12321131/diff/69001/build/android/find...
File build/android/findbugs_filter/findbugs_known_bugs.txt (right):

https://chromiumcodereview.appspot.com/12321131/diff/69001/build/android/find...
build/android/findbugs_filter/findbugs_known_bugs.txt:69: M D REC: Exception is
caught when Exception is not thrown in
org.chromium.content.browser.TextInputTypeTest.isInputVisible(ContentView)  At
TextInputTypeTest.java
This should be removed. I finally figured out why it's here. This test in the
downstream repo has the wrong package name so if you have that file in your
source tree, it erroneously shows up here.

[kjyoun, 2013-03-18 08:35:30]

Fixed known bugs in ChildProcessService.java, ChildProcessConnection.java and
ChildProcessLauncher.java

Ready to submit.

On 2013/03/15 16:40:37, Yaron wrote:
> Looks like my warnings change is insufficient. You can either land with
dcommit
> or we need to fix the findbugs issues
> 
>
https://chromiumcodereview.appspot.com/12321131/diff/69001/build/android/find...
> File build/android/findbugs_filter/findbugs_known_bugs.txt (right):
> 
>
https://chromiumcodereview.appspot.com/12321131/diff/69001/build/android/find...
> build/android/findbugs_filter/findbugs_known_bugs.txt:69: M D REC: Exception
is
> caught when Exception is not thrown in
> org.chromium.content.browser.TextInputTypeTest.isInputVisible(ContentView)  At
> TextInputTypeTest.java
> This should be removed. I finally figured out why it's here. This test in the
> downstream repo has the wrong package name so if you have that file in your
> source tree, it erroneously shows up here.

[commit-bot: I haz the power, 2013-03-18 20:34:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kjyoun@google.com/12321131/62003

[commit-bot: I haz the power, 2013-03-18 20:34:29]

Failed to apply patch for build/android/findbugs_filter/findbugs_known_bugs.txt:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file build/android/findbugs_filter/findbugs_known_bugs.txt
  Hunk #3 succeeded at 31 with fuzz 2 (offset -29 lines).
  Hunk #4 FAILED at 74.
  1 out of 4 hunks FAILED -- saving rejects to file
build/android/findbugs_filter/findbugs_known_bugs.txt.rej

Patch:       build/android/findbugs_filter/findbugs_known_bugs.txt
Index: build/android/findbugs_filter/findbugs_known_bugs.txt
diff --git a/build/android/findbugs_filter/findbugs_known_bugs.txt
b/build/android/findbugs_filter/findbugs_known_bugs.txt
index
5f5f25954f3e489fb76c7f25758b393739406f08..490ab751cff596f409ca1b828e8e59189933246f
100644
--- a/build/android/findbugs_filter/findbugs_known_bugs.txt
+++ b/build/android/findbugs_filter/findbugs_known_bugs.txt
@@ -1,5 +1,4 @@
 H C EC: Using pointer equality to compare a JavaBridgeCoercionTest$CustomType
with a JavaBridgeCoercionTest$CustomType2 in
org.chromium.content.browser.JavaBridgeCoercionTest.testPassJavaObject()  At
JavaBridgeCoercionTest.java
-H D RCN: Redundant nullcheck of
org.chromium.content.browser.SandboxedProcessConnection.mConnectionParams, which
is known to be non-null in
org.chromium.content.browser.SandboxedProcessConnection.doConnectionSetup() 
Redundant null check at SandboxedProcessConnection.java
 M B DE: org.chromium.net.X509Util.clearTestRootCertificates() might ignore
java.io.IOException  At X509Util.java
 M B Nm: The method name
org.chromium.base.test.util.ScalableTimeout.ScaleTimeout(long) doesn't start
with a lower case letter  At ScalableTimeout.java
 M B RV: exceptional return value of java.io.File.delete() ignored in
org.chromium.android_webview.test.ArchiveTest.doArchiveTest(AwContents, String,
boolean, String)  At ArchiveTest.java
@@ -17,8 +16,6 @@ M C CSM: Shouldn't use synchronized method, please narrow down
the synchronizati
 M C CSM: Shouldn't use synchronized method, please narrow down the
synchronization scope.  At JavaBridgeReturnValuesTest.java
 M C CSM: Shouldn't use synchronized method, please narrow down the
synchronization scope.  At JavaBridgeTestBase.java
 M C CSM: Shouldn't use synchronized method, please narrow down the
synchronization scope.  At PerfTraceEvent.java
-M C CSM: Shouldn't use synchronized method, please narrow down the
synchronization scope.  At SandboxedProcessConnection.java
-M C CSM: Shouldn't use synchronized method, please narrow down the
synchronization scope.  At SandboxedProcessLauncher.java
 M C CSM: Shouldn't use synchronized method, please narrow down the
synchronization scope.  At SimpleSynchronizedMethod.java
 M C CSM: Shouldn't use synchronized method, please narrow down the
synchronization scope.  At SimpleSynchronizedStaticMethod.java
 M C CSM: Shouldn't use synchronized method, please narrow down the
synchronization scope.  At TraceEvent.java
@@ -63,7 +60,6 @@ M D SF: Switch statement found in
org.chromium.chrome.browser.database.SQLiteCur
 M D SF: Switch statement found in
org.chromium.content.browser.ContentSettings$EventHandler$1.handleMessage(Message)
where default case is missing  At ContentSettings.java
 M D SF: Switch statement found in
org.chromium.content.browser.HandleView.onTouchEvent(MotionEvent) where default
case is missing  At HandleView.java
 M D SF: Switch statement found in
org.chromium.content.browser.third_party.GestureDetector.onTouchEvent(MotionEvent)
where default case is missing  At GestureDetector.java
-M D ST: Write to static field
org.chromium.content.app.SandboxedProcessService.sContext from instance method
org.chromium.content.app.SandboxedProcessService.onCreate()  At
SandboxedProcessService.java
 M D ST: Write to static field
org.chromium.content.browser.ContentSettings.sAppCachePathIsSet from instance
method org.chromium.content.browser.ContentSettings.setAppCachePath(String)  At
ContentSettings.java
 M D UrF: Unread public/protected field:
org.chromium.content.browser.JavaBridgeBasicsTest$21.field  At
JavaBridgeBasicsTest.java
 M D UrF: Unread public/protected field:
org.chromium.content.browser.JavaBridgeFieldsTest$TestObject.booleanField  At
JavaBridgeFieldsTest.java
@@ -78,9 +74,6 @@ M D UrF: Unread public/protected field:
org.chromium.content.browser.JavaBridgeF
 M D UrF: Unread public/protected field:
org.chromium.content.browser.JavaBridgeFieldsTest$TestObject.shortField  At
JavaBridgeFieldsTest.java
 M D UrF: Unread public/protected field:
org.chromium.content.browser.JavaBridgeFieldsTest$TestObject.stringField  At
JavaBridgeFieldsTest.java
 M D UuF: Unused public or protected field:
org.chromium.content.browser.JavaBridgeBasicsTest$19.field  In
JavaBridgeBasicsTest.java
-M M IS: Inconsistent synchronization of
org.chromium.content.browser.SandboxedProcessConnection.mPID; locked 66% of time
 Unsynchronized access at SandboxedProcessConnection.java
-M M IS: Inconsistent synchronization of
org.chromium.content.browser.SandboxedProcessConnection.mService; locked 55% of
time  Unsynchronized access at SandboxedProcessConnection.java
-M M IS: Inconsistent synchronization of
org.chromium.content.browser.SandboxedProcessConnection.mServiceConnectComplete;
locked 60% of time  Unsynchronized access at SandboxedProcessConnection.java
 M M LI: Incorrect lazy initialization and update of static field
org.chromium.base.SystemMonitor.sInstance in
org.chromium.base.SystemMonitor.create(Context)  At SystemMonitor.java
 M M LI: Incorrect lazy initialization and update of static field
org.chromium.content.browser.ContentVideoView.sContentVideoView in
org.chromium.content.browser.ContentVideoView.createContentVideoView(int)  At
ContentVideoView.java
 M M UG:
org.chromium.content.browser.JavaBridgeReturnValuesTest$TestObject.getBooleanValue()
is unsynchronized,
org.chromium.content.browser.JavaBridgeReturnValuesTest$TestObject.setBooleanValue(boolean)
is synchronized  At JavaBridgeReturnValuesTest.java

[kjyoun, 2013-03-18 20:42:53]

Umm... It is still failing, though I fixed known bugs that I found related
with my code change.
This might be related with new known bugs from other changes.
(I manually deleted new known_bugs generated from latest code base that are
not related with my change and submitted the change)

Can you let me know how to do "dcommit"?
and do I have permission to do "dcommit"?


On Tue, Mar 19, 2013 at 5:34 AM, <commit-bot@chromium.org> wrote:

> Failed to apply patch for build/android/findbugs_filter/**
> findbugs_known_bugs.txt:
> While running patch -p1 --forward --force --no-backup-if-mismatch;
>   patching file build/android/findbugs_filter/**findbugs_known_bugs.txt
>   Hunk #3 succeeded at 31 with fuzz 2 (offset -29 lines).
>   Hunk #4 FAILED at 74.
>   1 out of 4 hunks FAILED -- saving rejects to file
> build/android/findbugs_filter/**findbugs_known_bugs.txt.rej
>
> Patch:       build/android/findbugs_filter/**findbugs_known_bugs.txt
> Index: build/android/findbugs_filter/**findbugs_known_bugs.txt
> diff --git a/build/android/findbugs_**filter/findbugs_known_bugs.txt
> b/build/android/findbugs_**filter/findbugs_known_bugs.txt
> index
> 5f5f25954f3e489fb76c7f25758b39**3739406f08..**
> 490ab751cff596f409ca1b828e8e59**189933246f
> 100644
> --- a/build/android/findbugs_**filter/findbugs_known_bugs.txt
> +++ b/build/android/findbugs_**filter/findbugs_known_bugs.txt
> @@ -1,5 +1,4 @@
>  H C EC: Using pointer equality to compare a JavaBridgeCoercionTest$**
> CustomType
> with a JavaBridgeCoercionTest$**CustomType2 in
> org.chromium.content.browser.**JavaBridgeCoercionTest.**testPassJavaObject()
>  At
> JavaBridgeCoercionTest.java
> -H D RCN: Redundant nullcheck of
>
org.chromium.content.browser.**SandboxedProcessConnection.**mConnectionParams,
> which
> is known to be non-null in
> org.chromium.content.browser.**SandboxedProcessConnection.**
> doConnectionSetup()
> Redundant null check at SandboxedProcessConnection.**java
>  M B DE: org.chromium.net.X509Util.**clearTestRootCertificates() might
> ignore
> java.io.IOException  At X509Util.java
>  M B Nm: The method name
> org.chromium.base.test.util.**ScalableTimeout.ScaleTimeout(**long)
> doesn't start
> with a lower case letter  At ScalableTimeout.java
>  M B RV: exceptional return value of java.io.File.delete() ignored in
> org.chromium.android_webview.**test.ArchiveTest.**doArchiveTest(AwContents,
> String,
> boolean, String)  At ArchiveTest.java
> @@ -17,8 +16,6 @@ M C CSM: Shouldn't use synchronized method, please
> narrow down
> the synchronizati
>  M C CSM: Shouldn't use synchronized method, please narrow down the
> synchronization scope.  At JavaBridgeReturnValuesTest.**java
>  M C CSM: Shouldn't use synchronized method, please narrow down the
> synchronization scope.  At JavaBridgeTestBase.java
>  M C CSM: Shouldn't use synchronized method, please narrow down the
> synchronization scope.  At PerfTraceEvent.java
> -M C CSM: Shouldn't use synchronized method, please narrow down the
> synchronization scope.  At SandboxedProcessConnection.**java
> -M C CSM: Shouldn't use synchronized method, please narrow down the
> synchronization scope.  At SandboxedProcessLauncher.java
>  M C CSM: Shouldn't use synchronized method, please narrow down the
> synchronization scope.  At SimpleSynchronizedMethod.java
>  M C CSM: Shouldn't use synchronized method, please narrow down the
> synchronization scope.  At SimpleSynchronizedStaticMethod**.java
>  M C CSM: Shouldn't use synchronized method, please narrow down the
> synchronization scope.  At TraceEvent.java
> @@ -63,7 +60,6 @@ M D SF: Switch statement found in
> org.chromium.chrome.browser.**database.SQLiteCur
>
>  M D SF: Switch statement found in
> org.chromium.content.browser.**ContentSettings$EventHandler$**
> 1.handleMessage(Message)
> where default case is missing  At ContentSettings.java
>
>  M D SF: Switch statement found in
> org.chromium.content.browser.**HandleView.onTouchEvent(**MotionEvent)
> where default
> case is missing  At HandleView.java
>
>  M D SF: Switch statement found in
> org.chromium.content.browser.**third_party.GestureDetector.**
> onTouchEvent(MotionEvent)
> where default case is missing  At GestureDetector.java
> -M D ST: Write to static field
> org.chromium.content.app.**SandboxedProcessService.**sContext from
> instance method
> org.chromium.content.app.**SandboxedProcessService.**onCreate()  At
> SandboxedProcessService.java
>  M D ST: Write to static field
> org.chromium.content.browser.**ContentSettings.**sAppCachePathIsSet from
> instance
> method
org.chromium.content.browser.**ContentSettings.**setAppCachePath(String)
>  At
> ContentSettings.java
>  M D UrF: Unread public/protected field:
> org.chromium.content.browser.**JavaBridgeBasicsTest$21.field  At
> JavaBridgeBasicsTest.java
>  M D UrF: Unread public/protected field:
> org.chromium.content.browser.**JavaBridgeFieldsTest$**TestObject.booleanField
>  At
> JavaBridgeFieldsTest.java
> @@ -78,9 +74,6 @@ M D UrF: Unread public/protected field:
> org.chromium.content.browser.**JavaBridgeF
>  M D UrF: Unread public/protected field:
> org.chromium.content.browser.**JavaBridgeFieldsTest$**TestObject.shortField
>  At
> JavaBridgeFieldsTest.java
>  M D UrF: Unread public/protected field:
> org.chromium.content.browser.**JavaBridgeFieldsTest$**TestObject.stringField
>  At
> JavaBridgeFieldsTest.java
>  M D UuF: Unused public or protected field:
> org.chromium.content.browser.**JavaBridgeBasicsTest$19.field  In
> JavaBridgeBasicsTest.java
> -M M IS: Inconsistent synchronization of
> org.chromium.content.browser.**SandboxedProcessConnection.**mPID; locked
> 66% of time
>  Unsynchronized access at SandboxedProcessConnection.**java
> -M M IS: Inconsistent synchronization of
> org.chromium.content.browser.**SandboxedProcessConnection.**mService;
> locked 55% of
> time  Unsynchronized access at SandboxedProcessConnection.**java
> -M M IS: Inconsistent synchronization of
> org.chromium.content.browser.**SandboxedProcessConnection.**
> mServiceConnectComplete;
> locked 60% of time  Unsynchronized access at SandboxedProcessConnection.**
> java
>  M M LI: Incorrect lazy initialization and update of static field
> org.chromium.base.**SystemMonitor.sInstance in
> org.chromium.base.**SystemMonitor.create(Context)  At SystemMonitor.java
>  M M LI: Incorrect lazy initialization and update of static field
> org.chromium.content.browser.**ContentVideoView.**sContentVideoView in
> org.chromium.content.browser.**ContentVideoView.**createContentVideoView(int)
>  At
> ContentVideoView.java
>  M M UG:
> org.chromium.content.browser.**JavaBridgeReturnValuesTest$**
> TestObject.getBooleanValue()
> is unsynchronized,
> org.chromium.content.browser.**JavaBridgeReturnValuesTest$**
> TestObject.setBooleanValue(**boolean)
> is synchronized  At JavaBridgeReturnValuesTest.**java
>
>
>
https://chromiumcodereview.**appspot.com/12321131/<https://chromiumcodereview...
>



-- 

KJ Youn/연규정 | Technical Solution Engineer |  kjyoun@google.com |
+82-10-8162-9397
Too brief? Here's why! http://emailcharter.org

[yfriedman, 2013-03-18 20:48:11]

I think you just need to rebase that. I removed a bunch of lines from
findbugs


On Mon, Mar 18, 2013 at 1:42 PM, KJ Youn <kjyoun@google.com> wrote:

> Umm... It is still failing, though I fixed known bugs that I found related
> with my code change.
> This might be related with new known bugs from other changes.
> (I manually deleted new known_bugs generated from latest code base that
> are not related with my change and submitted the change)
>
> Can you let me know how to do "dcommit"?
> and do I have permission to do "dcommit"?
>
>
> On Tue, Mar 19, 2013 at 5:34 AM, <commit-bot@chromium.org> wrote:
>
>> Failed to apply patch for build/android/findbugs_filter/**
>> findbugs_known_bugs.txt:
>> While running patch -p1 --forward --force --no-backup-if-mismatch;
>>   patching file build/android/findbugs_filter/**findbugs_known_bugs.txt
>>   Hunk #3 succeeded at 31 with fuzz 2 (offset -29 lines).
>>   Hunk #4 FAILED at 74.
>>   1 out of 4 hunks FAILED -- saving rejects to file
>> build/android/findbugs_filter/**findbugs_known_bugs.txt.rej
>>
>> Patch:       build/android/findbugs_filter/**findbugs_known_bugs.txt
>> Index: build/android/findbugs_filter/**findbugs_known_bugs.txt
>> diff --git a/build/android/findbugs_**filter/findbugs_known_bugs.txt
>> b/build/android/findbugs_**filter/findbugs_known_bugs.txt
>> index
>> 5f5f25954f3e489fb76c7f25758b39**3739406f08..**
>> 490ab751cff596f409ca1b828e8e59**189933246f
>> 100644
>> --- a/build/android/findbugs_**filter/findbugs_known_bugs.txt
>> +++ b/build/android/findbugs_**filter/findbugs_known_bugs.txt
>> @@ -1,5 +1,4 @@
>>  H C EC: Using pointer equality to compare a JavaBridgeCoercionTest$**
>> CustomType
>> with a JavaBridgeCoercionTest$**CustomType2 in
>> org.chromium.content.browser.**JavaBridgeCoercionTest.**testPassJavaObject()
>>  At
>> JavaBridgeCoercionTest.java
>> -H D RCN: Redundant nullcheck of
>>
org.chromium.content.browser.**SandboxedProcessConnection.**mConnectionParams,
>> which
>> is known to be non-null in
>> org.chromium.content.browser.**SandboxedProcessConnection.**
>> doConnectionSetup()
>> Redundant null check at SandboxedProcessConnection.**java
>>  M B DE: org.chromium.net.X509Util.**clearTestRootCertificates() might
>> ignore
>> java.io.IOException  At X509Util.java
>>  M B Nm: The method name
>> org.chromium.base.test.util.**ScalableTimeout.ScaleTimeout(**long)
>> doesn't start
>> with a lower case letter  At ScalableTimeout.java
>>  M B RV: exceptional return value of java.io.File.delete() ignored in
>> org.chromium.android_webview.**test.ArchiveTest.**doArchiveTest(AwContents,
>> String,
>> boolean, String)  At ArchiveTest.java
>> @@ -17,8 +16,6 @@ M C CSM: Shouldn't use synchronized method, please
>> narrow down
>> the synchronizati
>>  M C CSM: Shouldn't use synchronized method, please narrow down the
>> synchronization scope.  At JavaBridgeReturnValuesTest.**java
>>  M C CSM: Shouldn't use synchronized method, please narrow down the
>> synchronization scope.  At JavaBridgeTestBase.java
>>  M C CSM: Shouldn't use synchronized method, please narrow down the
>> synchronization scope.  At PerfTraceEvent.java
>> -M C CSM: Shouldn't use synchronized method, please narrow down the
>> synchronization scope.  At SandboxedProcessConnection.**java
>> -M C CSM: Shouldn't use synchronized method, please narrow down the
>> synchronization scope.  At SandboxedProcessLauncher.java
>>  M C CSM: Shouldn't use synchronized method, please narrow down the
>> synchronization scope.  At SimpleSynchronizedMethod.java
>>  M C CSM: Shouldn't use synchronized method, please narrow down the
>> synchronization scope.  At SimpleSynchronizedStaticMethod**.java
>>  M C CSM: Shouldn't use synchronized method, please narrow down the
>> synchronization scope.  At TraceEvent.java
>> @@ -63,7 +60,6 @@ M D SF: Switch statement found in
>> org.chromium.chrome.browser.**database.SQLiteCur
>>
>>  M D SF: Switch statement found in
>> org.chromium.content.browser.**ContentSettings$EventHandler$**
>> 1.handleMessage(Message)
>> where default case is missing  At ContentSettings.java
>>
>>  M D SF: Switch statement found in
>> org.chromium.content.browser.**HandleView.onTouchEvent(**MotionEvent)
>> where default
>> case is missing  At HandleView.java
>>
>>  M D SF: Switch statement found in
>> org.chromium.content.browser.**third_party.GestureDetector.**
>> onTouchEvent(MotionEvent)
>> where default case is missing  At GestureDetector.java
>> -M D ST: Write to static field
>> org.chromium.content.app.**SandboxedProcessService.**sContext from
>> instance method
>> org.chromium.content.app.**SandboxedProcessService.**onCreate()  At
>> SandboxedProcessService.java
>>  M D ST: Write to static field
>> org.chromium.content.browser.**ContentSettings.**sAppCachePathIsSet from
>> instance
>> method
org.chromium.content.browser.**ContentSettings.**setAppCachePath(String)
>>  At
>> ContentSettings.java
>>  M D UrF: Unread public/protected field:
>> org.chromium.content.browser.**JavaBridgeBasicsTest$21.field  At
>> JavaBridgeBasicsTest.java
>>  M D UrF: Unread public/protected field:
>>
org.chromium.content.browser.**JavaBridgeFieldsTest$**TestObject.booleanField
>>  At
>> JavaBridgeFieldsTest.java
>> @@ -78,9 +74,6 @@ M D UrF: Unread public/protected field:
>> org.chromium.content.browser.**JavaBridgeF
>>  M D UrF: Unread public/protected field:
>> org.chromium.content.browser.**JavaBridgeFieldsTest$**TestObject.shortField
>>  At
>> JavaBridgeFieldsTest.java
>>  M D UrF: Unread public/protected field:
>> org.chromium.content.browser.**JavaBridgeFieldsTest$**TestObject.stringField
>>  At
>> JavaBridgeFieldsTest.java
>>  M D UuF: Unused public or protected field:
>> org.chromium.content.browser.**JavaBridgeBasicsTest$19.field  In
>> JavaBridgeBasicsTest.java
>> -M M IS: Inconsistent synchronization of
>> org.chromium.content.browser.**SandboxedProcessConnection.**mPID; locked
>> 66% of time
>>  Unsynchronized access at SandboxedProcessConnection.**java
>> -M M IS: Inconsistent synchronization of
>> org.chromium.content.browser.**SandboxedProcessConnection.**mService;
>> locked 55% of
>> time  Unsynchronized access at SandboxedProcessConnection.**java
>> -M M IS: Inconsistent synchronization of
>> org.chromium.content.browser.**SandboxedProcessConnection.**
>> mServiceConnectComplete;
>> locked 60% of time  Unsynchronized access at SandboxedProcessConnection.*
>> *java
>>  M M LI: Incorrect lazy initialization and update of static field
>> org.chromium.base.**SystemMonitor.sInstance in
>> org.chromium.base.**SystemMonitor.create(Context)  At SystemMonitor.java
>>  M M LI: Incorrect lazy initialization and update of static field
>> org.chromium.content.browser.**ContentVideoView.**sContentVideoView in
>>
org.chromium.content.browser.**ContentVideoView.**createContentVideoView(int)
>>  At
>> ContentVideoView.java
>>  M M UG:
>> org.chromium.content.browser.**JavaBridgeReturnValuesTest$**
>> TestObject.getBooleanValue()
>> is unsynchronized,
>> org.chromium.content.browser.**JavaBridgeReturnValuesTest$**
>> TestObject.setBooleanValue(**boolean)
>> is synchronized  At JavaBridgeReturnValuesTest.**java
>>
>>
>>
https://chromiumcodereview.**appspot.com/12321131/<https://chromiumcodereview...
>>
>
>
>
> --
>
> KJ Youn/연규정 | Technical Solution Engineer |  kjyoun@google.com |
+82-10-8162-9397
> Too brief? Here's why! http://emailcharter.org
>

[commit-bot: I haz the power, 2013-03-18 20:55:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kjyoun@google.com/12321131/86043

[kjyoun, 2013-03-18 20:59:07]

Eventually buildbot is launched to try, passing presubmit check.
Thanks Yaron.

When this change is landed on, we have to apply matching change under clank as
well.
https://gerrit-int.chromium.org/#/c/33418/

On 2013/03/18 20:55:31, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
> https://chromium-status.appspot.com/cq/kjyoun%40google.com/12321131/86043

[commit-bot: I haz the power, 2013-03-19 01:43:38]

Change committed as 188907