Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(257)

Issue 11748015: [Android WebView] Close InputStreams when we're done with them. (Closed)

Created:
7 years, 11 months ago by benm (inactive)
Modified:
7 years, 11 months ago
CC:
chromium-reviews, android-webview-reviews_chromium.org
Visibility:
Public.

Description

[Android WebView] Close InputStreams when we're done with them. When the native side of a Java Input Stream is destroyed, invoke the Java side close() method. Android only, android bots green NOTRY=true Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=174967

Patch Set 1 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -0 lines) Patch
M android_webview/native/input_stream_impl.cc View 2 chunks +4 lines, -0 lines 5 comments Download

Messages

Total messages: 10 (0 generated)
benm (inactive)
ptal
7 years, 11 months ago (2013-01-03 14:56:16 UTC) #1
mkosiba (inactive)
LGTM Was that a strict mode error or something?
7 years, 11 months ago (2013-01-03 15:08:31 UTC) #2
benm (inactive)
On 2013/01/03 15:08:31, Martin Kosiba wrote: > LGTM > > Was that a strict mode ...
7 years, 11 months ago (2013-01-03 15:38:00 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benm@chromium.org/11748015/1
7 years, 11 months ago (2013-01-03 15:38:11 UTC) #4
commit-bot: I haz the power
Change committed as 174967
7 years, 11 months ago (2013-01-03 16:26:40 UTC) #5
joth
Thanks for fixing this warning. https://codereview.chromium.org/11748015/diff/1/android_webview/native/input_stream_impl.cc File android_webview/native/input_stream_impl.cc (right): https://codereview.chromium.org/11748015/diff/1/android_webview/native/input_stream_impl.cc#newcode51 android_webview/native/input_stream_impl.cc:51: DCHECK(!ClearException(env)); the ClearException will ...
7 years, 11 months ago (2013-01-03 19:18:18 UTC) #6
mkosiba (inactive)
https://codereview.chromium.org/11748015/diff/1/android_webview/native/input_stream_impl.cc File android_webview/native/input_stream_impl.cc (right): https://codereview.chromium.org/11748015/diff/1/android_webview/native/input_stream_impl.cc#newcode51 android_webview/native/input_stream_impl.cc:51: DCHECK(!ClearException(env)); uh, actually due to the way the JNI ...
7 years, 11 months ago (2013-01-04 12:31:11 UTC) #7
benm (inactive)
https://codereview.chromium.org/11748015/diff/1/android_webview/native/input_stream_impl.cc File android_webview/native/input_stream_impl.cc (right): https://codereview.chromium.org/11748015/diff/1/android_webview/native/input_stream_impl.cc#newcode51 android_webview/native/input_stream_impl.cc:51: DCHECK(!ClearException(env)); I see. So right now in the exception ...
7 years, 11 months ago (2013-01-04 12:55:11 UTC) #8
mkosiba (inactive)
https://codereview.chromium.org/11748015/diff/1/android_webview/native/input_stream_impl.cc File android_webview/native/input_stream_impl.cc (right): https://codereview.chromium.org/11748015/diff/1/android_webview/native/input_stream_impl.cc#newcode51 android_webview/native/input_stream_impl.cc:51: DCHECK(!ClearException(env)); On 2013/01/04 12:55:11, benm wrote: > I see. ...
7 years, 11 months ago (2013-01-04 14:41:01 UTC) #9
benm (inactive)
7 years, 11 months ago (2013-01-04 16:59:40 UTC) #10
Message was sent while issue was closed.
https://codereview.chromium.org/11748015/diff/1/android_webview/native/input_...
File android_webview/native/input_stream_impl.cc (right):

https://codereview.chromium.org/11748015/diff/1/android_webview/native/input_...
android_webview/native/input_stream_impl.cc:51: DCHECK(!ClearException(env));
Gotcha. Follow up in https://codereview.chromium.org/11748032/.

Powered by Google App Engine
This is Rietveld 408576698