[WebView] Replace AwContentsIoThreadClient, InputStream and AwWebResourceResponse

[WebView] Replace AwContentsIoThreadClient, InputStream and AwWebResourceResponse

This CL:

1) Replaced AwContentsIoThreadClient with AwContentsIoThreadClientImpl
2) Replaced InputStream with InputStreamImpl
3) Replaced AwWebResourceResponse with AwWebResourceResponseImpl, and
   moved implementation into browser/net/.

Replacing them due to the merge of native/ and browser/

BUG=716604
Review-Url: https://codereview.chromium.org/2889193004
Cr-Commit-Position: refs/heads/master@{#473769}
Committed: https://chromium.googlesource.com/chromium/src/+/e9928ffd65c7b5def5a8a5af659ab647f437e3b8

[Shimi Zhang, 2017-05-19 23:12:35]

The CQ bit was checked by ctzsm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-19 23:13:29]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Shimi Zhang, 2017-05-19 23:55:46]

Description was changed from

==========
[WebView] Replace AwContentsIoThreadClient and InputStream

This CL:

1) Replaced AwContentsIoThreadClient with AwContentsIoThreadClientImpl
2) Replaced InputStream with InputStreamImpl

Both due to the merge of native/ and browser/, with some cleanup.

BUG=716604
==========

to

==========
[WebView] Replace AwContentsIoThreadClient, InputStream and
AwWebResourceResponse

This CL:

1) Replaced AwContentsIoThreadClient with AwContentsIoThreadClientImpl
2) Replaced InputStream with InputStreamImpl
3) Replaced AwWebResourceResponse with AwWebResourceResponseImpl, and
   moved implementation into browser/net/.

Replacing them due to the merge of native/ and browser/

BUG=716604
==========

[Shimi Zhang, 2017-05-19 23:56:00]

The CQ bit was checked by ctzsm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-19 23:56:30]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-20 00:53:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-20 00:53:16]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Shimi Zhang, 2017-05-22 23:11:01]

The CQ bit was checked by ctzsm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-22 23:11:43]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Shimi Zhang, 2017-05-22 23:23:03]

ctzsm@chromium.org changed reviewers:
+ boliu@chromium.org

[Shimi Zhang, 2017-05-22 23:23:03]

Bo, PTAL, thanks!

[sgurun-gerrit only, 2017-05-22 23:24:05]

On 2017/05/22 23:23:03, Shimi Zhang wrote:
> Bo, PTAL, thanks!

omg, 700 lines deleted net, if the stats above are correct :)

[boliu, 2017-05-22 23:27:15]

On 2017/05/22 23:24:05, sgurun wrote:
> On 2017/05/22 23:23:03, Shimi Zhang wrote:
> > Bo, PTAL, thanks!
> 
> omg, 700 lines deleted net, if the stats above are correct :)

they are not correct :p

moved file counts as all lines deleted, but A+ files count as modification

[sgurun-gerrit only, 2017-05-22 23:28:05]

On 2017/05/22 23:27:15, boliu wrote:
> On 2017/05/22 23:24:05, sgurun wrote:
> > On 2017/05/22 23:23:03, Shimi Zhang wrote:
> > > Bo, PTAL, thanks!
> > 
> > omg, 700 lines deleted net, if the stats above are correct :)
> 
> they are not correct :p
> 
> moved file counts as all lines deleted, but A+ files count as modification

LOL

[Shimi Zhang, 2017-05-22 23:29:55]

On 2017/05/22 23:28:05, sgurun wrote:
> On 2017/05/22 23:27:15, boliu wrote:
> > On 2017/05/22 23:24:05, sgurun wrote:
> > > On 2017/05/22 23:23:03, Shimi Zhang wrote:
> > > > Bo, PTAL, thanks!
> > > 
> > > omg, 700 lines deleted net, if the stats above are correct :)
> > 
> > they are not correct :p
> > 
> > moved file counts as all lines deleted, but A+ files count as modification
> 
> LOL

Unfortunately, the correct stats on my local is 15 files changed, 158
insertions(+), 287 deletions(-), :P

[boliu, 2017-05-22 23:30:11]

https://codereview.chromium.org/2889193004/diff/40001/android_webview/browser...
File android_webview/browser/input_stream.h (right):

https://codereview.chromium.org/2889193004/diff/40001/android_webview/browser...
android_webview/browser/input_stream.h:32: virtual ~InputStream();
does this still need virtual?

[Shimi Zhang, 2017-05-22 23:36:33]

On 2017/05/22 23:30:11, boliu wrote:
>
https://codereview.chromium.org/2889193004/diff/40001/android_webview/browser...
> File android_webview/browser/input_stream.h (right):
> 
>
https://codereview.chromium.org/2889193004/diff/40001/android_webview/browser...
> android_webview/browser/input_stream.h:32: virtual ~InputStream();
> does this still need virtual?

We still need it,
https://cs.chromium.org/chromium/src/android_webview/browser/net/android_stre...
and is overriding it as an empty destructor, I think we could keep virtual there
to tell someone is overriding it.

[boliu, 2017-05-22 23:38:40]

On 2017/05/22 23:36:33, Shimi Zhang wrote:
> On 2017/05/22 23:30:11, boliu wrote:
> >
>
https://codereview.chromium.org/2889193004/diff/40001/android_webview/browser...
> > File android_webview/browser/input_stream.h (right):
> > 
> >
>
https://codereview.chromium.org/2889193004/diff/40001/android_webview/browser...
> > android_webview/browser/input_stream.h:32: virtual ~InputStream();
> > does this still need virtual?
> 
> We still need it,
>
https://cs.chromium.org/chromium/src/android_webview/browser/net/android_stre...
> and is overriding it as an empty destructor, I think we could keep virtual
there
> to tell someone is overriding it.

I was asking more generally. In general, empty destructor does not imply no-op
destructor. All the complex types needs to be destroyed too.

lgtm

[Shimi Zhang, 2017-05-22 23:51:19]

> I was asking more generally. In general, empty destructor does not imply no-op
> destructor. All the complex types needs to be destroyed too.
> 
> lgtm

From my understanding, even it is an empty destructor, parent destructor will be
invoked, is that what you mean does not imply no-op destructor?

[boliu, 2017-05-23 00:34:15]

On 2017/05/22 23:51:19, Shimi Zhang wrote:
> > I was asking more generally. In general, empty destructor does not imply
no-op
> > destructor. All the complex types needs to be destroyed too.
> > 
> > lgtm
> 
> From my understanding, even it is an empty destructor, parent destructor will
be
> invoked, is that what you mean does not imply no-op destructor?

We are talking about different issues.

I was just saying just because the body of destructor is empty, does not mean
there is no destructor.

But you are right. If an object is deleted using a child pointer, then parent
destructor will be called.

But regardless, that kind of usage should be avoided because  deleting an object
with the parent pointer will cause undefined behavior. So the general rule is
keep destructor virtual if it has subclasses.

[commit-bot: I haz the power, 2017-05-23 00:47:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-23 00:47:06]

Dry run: This issue passed the CQ dry run.

[sgurun-gerrit only, 2017-05-23 00:49:26]

On 2017/05/23 00:34:15, boliu wrote:
> On 2017/05/22 23:51:19, Shimi Zhang wrote:
> > > I was asking more generally. In general, empty destructor does not imply
> no-op
> > > destructor. All the complex types needs to be destroyed too.
> > > 
> > > lgtm
> > 
> > From my understanding, even it is an empty destructor, parent destructor
will
> be
> > invoked, is that what you mean does not imply no-op destructor?
> 
> We are talking about different issues.
> 
> I was just saying just because the body of destructor is empty, does not mean
> there is no destructor.
> 
> But you are right. If an object is deleted using a child pointer, then parent
> destructor will be called.
> 
> But regardless, that kind of usage should be avoided because  deleting an
object
> with the parent pointer will cause undefined behavior. So the general rule is
> keep destructor virtual if it has subclasses.

Google code conventions:
"Make your destructor virtual if necessary. If your class has virtual methods,
its destructor should be virtual."

[Shimi Zhang, 2017-05-23 00:49:51]

The CQ bit was checked by ctzsm@chromium.org

[commit-bot: I haz the power, 2017-05-23 00:50:33]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-23 00:56:54]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495500591258440,
"parent_rev": "a252bab61a3811269fdca54ca84083856a7e2934", "commit_rev":
"e9928ffd65c7b5def5a8a5af659ab647f437e3b8"}

[commit-bot: I haz the power, 2017-05-23 00:57:09]

Description was changed from

==========
[WebView] Replace AwContentsIoThreadClient, InputStream and
AwWebResourceResponse

This CL:

1) Replaced AwContentsIoThreadClient with AwContentsIoThreadClientImpl
2) Replaced InputStream with InputStreamImpl
3) Replaced AwWebResourceResponse with AwWebResourceResponseImpl, and
   moved implementation into browser/net/.

Replacing them due to the merge of native/ and browser/

BUG=716604
==========

to

==========
[WebView] Replace AwContentsIoThreadClient, InputStream and
AwWebResourceResponse

This CL:

1) Replaced AwContentsIoThreadClient with AwContentsIoThreadClientImpl
2) Replaced InputStream with InputStreamImpl
3) Replaced AwWebResourceResponse with AwWebResourceResponseImpl, and
   moved implementation into browser/net/.

Replacing them due to the merge of native/ and browser/

BUG=716604

Review-Url: https://codereview.chromium.org/2889193004
Cr-Commit-Position: refs/heads/master@{#473769}
Committed:
https://chromium.googlesource.com/chromium/src/+/e9928ffd65c7b5def5a8a5af659a...
==========

[commit-bot: I haz the power, 2017-05-23 00:57:10]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/e9928ffd65c7b5def5a8a5af659a...