Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1809013002:
Fix webview memory leak when keyboard was shown (Closed)
|
Created:
4 years, 9 months ago by Changwan Ryu Modified:
4 years, 9 months ago CC:
chromium-reviews, darin-cc_chromium.org, jam, android-webview-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix webview memory leak when keyboard was shown
Android framework does not seem to release ResultReceiver passed in
InputMethodManager#showSoftInput() after use. As a result, WebView
will not be garbage collected after it's removed, if keyboard shows up
on the screen once.
ResultReceiver cannot be avoided since we want to scroll to the editable
node only after input method window shows up.
This is a fix to weak-reference CVC object (and thus WebView as well) from
ResultReceiver. Unfortunately, ResultReceiver will still be leaked,
but the leak can be significantly reduced.
A test is added to AwContentsGarbageCollectionTest to test that references
to WebView can be removed even when showSoftInput() is called before
garbage collection.
Also, I've manually tested that the new test can catch the memory leak
issue when we strong-reference CVC.
BUG=595613
Committed: https://crrev.com/fcc3fbf1652285b2a89eabd092d35d0fdf199ac2
Cr-Commit-Position: refs/heads/master@{#381891}
Patch Set 1 #Patch Set 2 : #Patch Set 3 : changed to weakreference approach #Patch Set 4 : fixed nits #Patch Set 5 : keep a single instance instead of leaking many #
Total comments: 4
Patch Set 6 : added comments and fixed nits #Patch Set 7 : removed thread.sleep from test #
Total comments: 2
Patch Set 8 : do not call showImeIfNeeded #
Total comments: 6
Patch Set 9 : address boliu@'s comments #Patch Set 10 : #
Total comments: 2
Patch Set 11 : removed final #Patch Set 12 : fixed nits #
Messages
Total messages: 37 (10 generated)
Description was changed from ========== Fix WebView leakage when keyboard was shown We call showSoftInput() with a ResultReceiver object as a parameter for the following reason: - OSK triggers resize of the viewport - If we scroll to the input form at the same time as viewport resizes, flickering may happen. - In order to avoid flickering, we delay scrolling until viewport resize finishes. However, with crbug.com/404315 gets resolved, we can remove the original delay logic and thus ResultReceiver. BUG=595613 ========== to ========== Fix webview memory leak when keyboard was shown Android framework does not seem to release ResultReceiver passed in InputMethodManager#showSoftInput() after use. As a result, WebView will not be garbage collected after it's removed, if keyboard shows up on the screen once. BUG=595613 ==========
changwan@chromium.org changed reviewers: + aelias@chromium.org, boliu@chromium.org, tedchoc@chromium.org
Description was changed from ========== Fix webview memory leak when keyboard was shown Android framework does not seem to release ResultReceiver passed in InputMethodManager#showSoftInput() after use. As a result, WebView will not be garbage collected after it's removed, if keyboard shows up on the screen once. BUG=595613 ========== to ========== Fix webview memory leak when keyboard was shown Android framework does not seem to release ResultReceiver passed in InputMethodManager#showSoftInput() after use. As a result, WebView will not be garbage collected after it's removed, if keyboard shows up on the screen once. We keep a WeakReference to ContentViewCore in ResultReceiver to avoid the leak as a workaround. BUG=595613 ==========
Did you actually test that ResultReceiver is the *only* reference that framework code holds, ie does this actually work? Also I'd like to understand this issue more before landing ths. At least point the finger to an android bug. https://chromiumcodereview.appspot.com/1809013002/diff/80001/android_webview/... File android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java (right): https://chromiumcodereview.appspot.com/1809013002/diff/80001/android_webview/... android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:122: containerView.getAwContents().getWebContents().showImeIfNeeded(); wrong thread https://chromiumcodereview.appspot.com/1809013002/diff/80001/android_webview/... android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:124: Thread.sleep(1000); No way. The flake this will cause is not worth the maintenance cost of having this test at all. I suggest test exactly what the CL is trying to fix, ie just hold a strong ref to ResultReceiver. I realize this won't catch future regressions, but neither will a disabled test.
Android has said that WeakReference is the right thing to do here. And I verified fix on trunk. So just make sure the test is not flaky, and maybe add an comment to explain the issue.
PTAL https://codereview.chromium.org/1809013002/diff/80001/android_webview/javates... File android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java (right): https://codereview.chromium.org/1809013002/diff/80001/android_webview/javates... android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:122: containerView.getAwContents().getWebContents().showImeIfNeeded(); On 2016/03/17 16:41:20, boliu wrote: > wrong thread Oops... Good catch. Fixed now. https://codereview.chromium.org/1809013002/diff/80001/android_webview/javates... android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:124: Thread.sleep(1000); On 2016/03/17 16:41:20, boliu wrote: > No way. The flake this will cause is not worth the maintenance cost of having > this test at all. > > I suggest test exactly what the CL is trying to fix, ie just hold a strong ref > to ResultReceiver. I realize this won't catch future regressions, but neither > will a disabled test. ResultReceiver may still be leaked due to the Android framework's design. The sleep is just there to increase chances to catch a memory leak when there is one (5/5 repro against pre-patch). I couldn't think of a better way to remove the sleep since we need to wait for OSK to show up. This is not going to be flaky as long as we don't have a memory leak (which is the current status with the fix.)
Description was changed from ========== Fix webview memory leak when keyboard was shown Android framework does not seem to release ResultReceiver passed in InputMethodManager#showSoftInput() after use. As a result, WebView will not be garbage collected after it's removed, if keyboard shows up on the screen once. We keep a WeakReference to ContentViewCore in ResultReceiver to avoid the leak as a workaround. BUG=595613 ========== to ========== Fix webview memory leak when keyboard was shown Android framework does not seem to release ResultReceiver passed in InputMethodManager#showSoftInput() after use. As a result, WebView will not be garbage collected after it's removed, if keyboard shows up on the screen once. ResultReceiver cannot be avoided since we want to scroll to the editable node only after input method window shows up. This is a fix to weak-reference CVC object (and thus WebView as well) from ResultReceiver. Unfortunately, ResultReceiver will still be leaked, but the leak can be significantly reduced. A test is added to AwContentsGarbageCollectionTest to test that references to WebView can be removed even when showSoftInput() is called before garbage collection. Also, I've manually tested that the new test can catch the memory leak issue before the patch is applied. BUG=595613 ==========
CVC - lgtm
On 2016/03/18 00:35:19, Ted C wrote: > CVC - lgtm
On 2016/03/18 00:40:27, boliu wrote: > On 2016/03/18 00:35:19, Ted C wrote: > > CVC - lgtm oops.. No, sleep is still not ok. The risk is test timeout on a slow device, since in test, you are wasting 2 seconds just sleeping. And you are bringing the keyboard up probably for the first time, which also takes time. I don't know what the timeout test for small test is, but it's fundamentally not ok imo. And making it large test is just paving over the problem. We'll need a reliable way to detect if keyboard is up, and stackoverflow only says use android:windowSoftInputMode=adjustResize and check for window resizes, which is terrible. Maybe this is just not testable at the integration level. I suggest just write a "unit" test: hold a strong ref to the ResultReceiver, and make sure everything else works.
On 2016/03/18 00:45:53, boliu wrote: > On 2016/03/18 00:40:27, boliu wrote: > > On 2016/03/18 00:35:19, Ted C wrote: > > > CVC - lgtm > > oops.. > > No, sleep is still not ok. The risk is test timeout on a slow device, since in > test, you are wasting 2 seconds just sleeping. And you are bringing the keyboard > up probably for the first time, which also takes time. I don't know what the > timeout test for small test is, but it's fundamentally not ok imo. And making it > large test is just paving over the problem. > > We'll need a reliable way to detect if keyboard is up, and stackoverflow only > says use android:windowSoftInputMode=adjustResize and check for window resizes, > which is terrible. Maybe this is just not testable at the integration level. > > I suggest just write a "unit" test: hold a strong ref to the ResultReceiver, and > make sure everything else works. Maybe we should land the fix first, and think about how to properly test this afterwards.
Description was changed from ========== Fix webview memory leak when keyboard was shown Android framework does not seem to release ResultReceiver passed in InputMethodManager#showSoftInput() after use. As a result, WebView will not be garbage collected after it's removed, if keyboard shows up on the screen once. ResultReceiver cannot be avoided since we want to scroll to the editable node only after input method window shows up. This is a fix to weak-reference CVC object (and thus WebView as well) from ResultReceiver. Unfortunately, ResultReceiver will still be leaked, but the leak can be significantly reduced. A test is added to AwContentsGarbageCollectionTest to test that references to WebView can be removed even when showSoftInput() is called before garbage collection. Also, I've manually tested that the new test can catch the memory leak issue before the patch is applied. BUG=595613 ========== to ========== Fix webview memory leak when keyboard was shown Android framework does not seem to release ResultReceiver passed in InputMethodManager#showSoftInput() after use. As a result, WebView will not be garbage collected after it's removed, if keyboard shows up on the screen once. ResultReceiver cannot be avoided since we want to scroll to the editable node only after input method window shows up. This is a fix to weak-reference CVC object (and thus WebView as well) from ResultReceiver. Unfortunately, ResultReceiver will still be leaked, but the leak can be significantly reduced. A test is added to AwContentsGarbageCollectionTest to test that references to WebView can be removed even when showSoftInput() is called before garbage collection. Also, I've manually tested that the new test can catch the memory leak issue when we strong-reference CVC. BUG=595613 ==========
On 2016/03/18 01:02:12, boliu wrote: > On 2016/03/18 00:45:53, boliu wrote: > > On 2016/03/18 00:40:27, boliu wrote: > > > On 2016/03/18 00:35:19, Ted C wrote: > > > > CVC - lgtm > > > > oops.. > > > > No, sleep is still not ok. The risk is test timeout on a slow device, since in > > test, you are wasting 2 seconds just sleeping. And you are bringing the > keyboard > > up probably for the first time, which also takes time. I don't know what the > > timeout test for small test is, but it's fundamentally not ok imo. And making > it > > large test is just paving over the problem. > > > > We'll need a reliable way to detect if keyboard is up, and stackoverflow only > > says use android:windowSoftInputMode=adjustResize and check for window > resizes, > > which is terrible. Maybe this is just not testable at the integration level. > > > > I suggest just write a "unit" test: hold a strong ref to the ResultReceiver, > and > > make sure everything else works. > > Maybe we should land the fix first, and think about how to properly test this > afterwards. PTAL
https://chromiumcodereview.appspot.com/1809013002/diff/120001/android_webview... File android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java (right): https://chromiumcodereview.appspot.com/1809013002/diff/120001/android_webview... android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:138: CriteriaHelper.pollForUIThreadCriteria(new Criteria() { Need to do this for both webviews, since we are not verifying which AwContents is getting gc-ed. Can the method on CVC just create the ResultReceiver without having to go through showImeIfNeeded stuff? getNewShowKeyboardReceiver can call that method too.
https://chromiumcodereview.appspot.com/1809013002/diff/120001/android_webview... File android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java (right): https://chromiumcodereview.appspot.com/1809013002/diff/120001/android_webview... android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:138: CriteriaHelper.pollForUIThreadCriteria(new Criteria() { On 2016/03/18 01:37:14, boliu wrote: > Need to do this for both webviews, since we are not verifying which AwContents > is getting gc-ed. > > Can the method on CVC just create the ResultReceiver without having to go > through showImeIfNeeded stuff? getNewShowKeyboardReceiver can call that method > too. Done.
https://chromiumcodereview.appspot.com/1809013002/diff/140001/android_webview... File android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java (right): https://chromiumcodereview.appspot.com/1809013002/diff/140001/android_webview... android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:110: public void testShowSoftKeyboardAndGcOnce() throws Throwable { can you rename this test to just about keyboard receiver? https://chromiumcodereview.appspot.com/1809013002/diff/140001/android_webview... android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:120: + "<input autofocus id%3D%22editor%22 %2F><%2Fform>"; about:blank should be ok now? https://chromiumcodereview.appspot.com/1809013002/diff/140001/android_webview... android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:130: CriteriaHelper.pollForUIThreadCriteria(new Criteria() { This doesn't need to be a poll anymore. You can use runTestOnUiThreadAndGetResult.
https://chromiumcodereview.appspot.com/1809013002/diff/140001/android_webview... File android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java (right): https://chromiumcodereview.appspot.com/1809013002/diff/140001/android_webview... android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:110: public void testShowSoftKeyboardAndGcOnce() throws Throwable { On 2016/03/18 01:58:52, boliu wrote: > can you rename this test to just about keyboard receiver? Changed to testHoldKeyboardResultReceiver. https://chromiumcodereview.appspot.com/1809013002/diff/140001/android_webview... android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:120: + "<input autofocus id%3D%22editor%22 %2F><%2Fform>"; On 2016/03/18 01:58:52, boliu wrote: > about:blank should be ok now? Done. https://chromiumcodereview.appspot.com/1809013002/diff/140001/android_webview... android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:130: CriteriaHelper.pollForUIThreadCriteria(new Criteria() { On 2016/03/18 01:58:52, boliu wrote: > This doesn't need to be a poll anymore. You can use > runTestOnUiThreadAndGetResult. Done.
lgtm https://chromiumcodereview.appspot.com/1809013002/diff/180001/android_webview... File android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java (right): https://chromiumcodereview.appspot.com/1809013002/diff/180001/android_webview... android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:115: final ResultReceiver resultReceivers[] = new ResultReceiver[MAX_IDLE_INSTANCES + 1]; don't need to be final
https://chromiumcodereview.appspot.com/1809013002/diff/180001/android_webview... File android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java (right): https://chromiumcodereview.appspot.com/1809013002/diff/180001/android_webview... android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java:115: final ResultReceiver resultReceivers[] = new ResultReceiver[MAX_IDLE_INSTANCES + 1]; On 2016/03/18 02:17:10, boliu wrote: > don't need to be final Done
tedchoc@, I added getNewShowKeyboardReceiver() method for testing after your lgtm. I'd like to submit this now as is, but please let me know if you're not ok with it. boliu@, thanks for the rigorous code review.
The CQ bit was checked by changwan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tedchoc@chromium.org, boliu@chromium.org Link to the patchset: https://chromiumcodereview.appspot.com/1809013002/#ps190003 (title: "fixed nits")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1809013002/190003 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1809013002/190003
Message was sent while issue was closed.
Description was changed from ========== Fix webview memory leak when keyboard was shown Android framework does not seem to release ResultReceiver passed in InputMethodManager#showSoftInput() after use. As a result, WebView will not be garbage collected after it's removed, if keyboard shows up on the screen once. ResultReceiver cannot be avoided since we want to scroll to the editable node only after input method window shows up. This is a fix to weak-reference CVC object (and thus WebView as well) from ResultReceiver. Unfortunately, ResultReceiver will still be leaked, but the leak can be significantly reduced. A test is added to AwContentsGarbageCollectionTest to test that references to WebView can be removed even when showSoftInput() is called before garbage collection. Also, I've manually tested that the new test can catch the memory leak issue when we strong-reference CVC. BUG=595613 ========== to ========== Fix webview memory leak when keyboard was shown Android framework does not seem to release ResultReceiver passed in InputMethodManager#showSoftInput() after use. As a result, WebView will not be garbage collected after it's removed, if keyboard shows up on the screen once. ResultReceiver cannot be avoided since we want to scroll to the editable node only after input method window shows up. This is a fix to weak-reference CVC object (and thus WebView as well) from ResultReceiver. Unfortunately, ResultReceiver will still be leaked, but the leak can be significantly reduced. A test is added to AwContentsGarbageCollectionTest to test that references to WebView can be removed even when showSoftInput() is called before garbage collection. Also, I've manually tested that the new test can catch the memory leak issue when we strong-reference CVC. BUG=595613 ==========
Message was sent while issue was closed.
Committed patchset #12 (id:190003)
Message was sent while issue was closed.
Description was changed from ========== Fix webview memory leak when keyboard was shown Android framework does not seem to release ResultReceiver passed in InputMethodManager#showSoftInput() after use. As a result, WebView will not be garbage collected after it's removed, if keyboard shows up on the screen once. ResultReceiver cannot be avoided since we want to scroll to the editable node only after input method window shows up. This is a fix to weak-reference CVC object (and thus WebView as well) from ResultReceiver. Unfortunately, ResultReceiver will still be leaked, but the leak can be significantly reduced. A test is added to AwContentsGarbageCollectionTest to test that references to WebView can be removed even when showSoftInput() is called before garbage collection. Also, I've manually tested that the new test can catch the memory leak issue when we strong-reference CVC. BUG=595613 ========== to ========== Fix webview memory leak when keyboard was shown Android framework does not seem to release ResultReceiver passed in InputMethodManager#showSoftInput() after use. As a result, WebView will not be garbage collected after it's removed, if keyboard shows up on the screen once. ResultReceiver cannot be avoided since we want to scroll to the editable node only after input method window shows up. This is a fix to weak-reference CVC object (and thus WebView as well) from ResultReceiver. Unfortunately, ResultReceiver will still be leaked, but the leak can be significantly reduced. A test is added to AwContentsGarbageCollectionTest to test that references to WebView can be removed even when showSoftInput() is called before garbage collection. Also, I've manually tested that the new test can catch the memory leak issue when we strong-reference CVC. BUG=595613 Committed: https://crrev.com/fcc3fbf1652285b2a89eabd092d35d0fdf199ac2 Cr-Commit-Position: refs/heads/master@{#381891} ==========
Message was sent while issue was closed.
Patchset 12 (id:??) landed as https://crrev.com/fcc3fbf1652285b2a89eabd092d35d0fdf199ac2 Cr-Commit-Position: refs/heads/master@{#381891}
Message was sent while issue was closed.
Hey, I feel that the CL description > (snip) ResultReceiver will still be leaked, > (snip) Unfortunately, ResultReceiver will still be leaked, are a bit misleading, because those objects are not permanently leaked but just require GCs in other processes.
Message was sent while issue was closed.
On 2016/03/18 05:07:44, yukawa wrote: > Hey, > > I feel that the CL description > > > (snip) ResultReceiver will still be leaked, > > (snip) Unfortunately, ResultReceiver will still be leaked, > > are a bit misleading, because those objects are not permanently leaked but just > require GCs in other processes. You're right. Sorry about the misleading description. Fortunately, the code has more precise comments than this description.
Message was sent while issue was closed.
On 2016/03/18 05:15:36, Changwan Ryu wrote: > On 2016/03/18 05:07:44, yukawa wrote: > > Hey, > > > > I feel that the CL description > > > > > (snip) ResultReceiver will still be leaked, > > > (snip) Unfortunately, ResultReceiver will still be leaked, > > > > are a bit misleading, because those objects are not permanently leaked but > just > > require GCs in other processes. > > You're right. > Sorry about the misleading description. Fortunately, the code has more precise > comments than this description. I think this comment isn't clear based on yukawa's description: https://code.google.com/p/chromium/codesearch#chromium/src/content/public/and... It isn't leaked, but its memory lifecycle is not in chrome's control, so we use a weakref to avoid tying our lifetime to the IME process lifetime.
Message was sent while issue was closed.
hush@chromium.org changed reviewers: + hush@chromium.org
Message was sent while issue was closed.
Hello, (i'm the sheriff) this CL causes our internal bot failure: ******************************************************************************** FindBugs run via: java -classpath /b/build/slave/clang-clankium-tot-builder/build/src/third_party/findbugs/lib/findbugs.jar: -Xmx768m -Dfindbugs.home="/b/build/slave/clang-clankium-tot-builder/build/src/third_party/findbugs" -jar /b/build/slave/clang-clankium-tot-builder/build/src/third_party/findbugs/lib/findbugs.jar -textui -sortByClass -pluginList /b/build/slave/clang-clankium-tot-builder/build/src/tools/android/findbugs_plugin/lib/chromiumPlugin.jar -xml:withMessages -auxclasspath /b/build/slave/clang-clankium-tot-builder/build/src/third_party/android_tools/sdk/platforms/android-23/android.jar:/b/build/slave/clang-clankium-tot-builder/build/src/third_party/android_tools/sdk/extras/android/support/multidex/library/libs/android-support-multidex.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/jsr_305_javalib.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/base_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/reporter_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/base_java_test_support.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/policy_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/policy_java_test_support.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/mojo_public_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/mojo_bindings_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/device_battery_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/device_bluetooth_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/device_usb_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/device_vibration_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/media_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/mojo_system_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/net_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/ui_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/third_party/android_tools/sdk/extras/android/support/v13/android-support-v13.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/blink_headers_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/capture_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/ui_accessibility_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/midi_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/protobuf_nano_javalib.jar:/b/build/slave/clang-clankium-tot-builder/build/src/third_party/cardboard-java/src/CardboardSample/libs/cardboard.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/content_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/content_java_test_support.jar:/b/build/slave/clang-clankium-tot-builder/build/src/third_party/android_tools/sdk/platforms/android-23/optional/org.apache.http.legacy.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/net_java_test_support.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/broker_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/external_video_surface_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/navigation_interception_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/web_contents_delegate_android_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/android_webview_java.jar:/b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/chromium_apk_android_webview_apk.jar -exclude /b/build/slave/clang-clankium-tot-builder/build/src/build/android/findbugs_filter/findbugs_exclude.xml /b/build/slave/clang-clankium-tot-builder/build/src/out/Debug/lib.java/chromium_apk_android_webview_test_apk.jar FindBugs reported the following issues: UC_USELESS_OBJECT: Useless object created In class org.chromium.android_webview.test.AwContentsGarbageCollectionTest In method org.chromium.android_webview.test.AwContentsGarbageCollectionTest.testHoldKeyboardResultReceiver() At AwContentsGarbageCollectionTest.java:[line 115] ******************************************************************************** It is also confusing what the problem is with findbugs. "resultReceivers" are used. So it is not useless.
Message was sent while issue was closed.
After reading the comments, it seems we need to add
@SuppressFBWarnings("UC_USELESS_OBJECT")
Message was sent while issue was closed.
Findbugs is complaining that that it's only assigned to once, but never used maybe? I think setting it to null at the end of the test might be good enough to silence this (considering containerViews) Findbugs isn't running anywhere upstream?
Message was sent while issue was closed.
On 2016/03/18 17:26:27, hush wrote:
> After reading the comments, it seems we need to add
> @SuppressFBWarnings("UC_USELESS_OBJECT")
That works too, with a comment.
Message was sent while issue was closed.
On 2016/03/18 17:26:27, hush wrote:
> After reading the comments, it seems we need to add
> @SuppressFBWarnings("UC_USELESS_OBJECT")
this fixes findbugs https://codereview.chromium.org/1815143003/
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
