Move MessagePort implementation from android_webview to content

Move MessagePort implementation from android_webview to content

-Rename all classes with AppWeb prefix instead of Aw
-Move browser side classes to content/browser/android
-Move renderer side class to content/renderer/android
-Add public interface in content for app_web_message_port_service
-Slim down message_port_provider now that all classes are in content
-Move all java classes to content/public/android with package
org.chromium.content.browser
-AppWebMessagePortMessageFilter ownership moves to RenderFrameHostImpl
-AppWebMessagePortClient ownership moves to RenderFrameImpl

BUG=651124
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/ed66fc0d1f1d7adb27862c6046212162445231ac
Cr-Commit-Position: refs/heads/master@{#424543}

[Yusuf, 2016-09-28 16:48:22]

Description was changed from

==========
Move MessagePort implementation from android_webview to content

-Rename all classes with AppWeb prefix instead of Aw
-Move browser side classes to content/browser/android
-Move renderer side class to content/renderer/android
-Add public interface in content for app_web_message_port_service
-Slim down message_port_provider now that all classes are in content
-Move all java classes to content/public/android with package
org.chromium.content.browser
-AppWebMessagePortMessageFilter ownership moves to RenderFrameHostImpl
-AppWebMessagePortClient ownership moves to RenderFrameImpl

BUG=651124
==========

to

==========
Move MessagePort implementation from android_webview to content

-Rename all classes with AppWeb prefix instead of Aw
-Move browser side classes to content/browser/android
-Move renderer side class to content/renderer/android
-Add public interface in content for app_web_message_port_service
-Slim down message_port_provider now that all classes are in content
-Move all java classes to content/public/android with package
org.chromium.content.browser
-AppWebMessagePortMessageFilter ownership moves to RenderFrameHostImpl
-AppWebMessagePortClient ownership moves to RenderFrameImpl

BUG=651124
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Yusuf, 2016-09-28 16:50:35]

yusufo@chromium.org changed reviewers:
+ jochen@chromium.org, sgurun@chromium.org

[Yusuf, 2016-09-28 16:50:36]

Here is the final version. 

This is intended to be only the move, which should keep the WebView side
implementation WAI and move the code to content. I think it would be much easier
to address any non-WebView and custom tabs specific concerns as follow-up.

PTAL!

[Yusuf, 2016-09-28 18:52:49]

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

[commit-bot: I haz the power, 2016-09-28 18:53:21]

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, 2016-09-28 19:43:58]

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

[commit-bot: I haz the power, 2016-09-28 19:43:59]

Dry run: This issue passed the CQ dry run.

[jochen (gone - plz use gerrit), 2016-09-29 12:08:17]

I wait for sgurun@ to finish the review first

[sgurun-gerrit only, 2016-09-29 15:26:32]

On 2016/09/29 12:08:17, jochen (slow) wrote:
> I wait for sgurun@ to finish the review first

ack, starting today afternoon, but won't get to complete in a day :)

[sgurun-gerrit only, 2016-09-30 23:40:59]

https://codereview.chromium.org/2375133002/diff/1/android_webview/lib/main/aw...
File android_webview/lib/main/aw_main_delegate.cc (right):

https://codereview.chromium.org/2375133002/diff/1/android_webview/lib/main/aw...
android_webview/lib/main/aw_main_delegate.cc:5: #include
<content/browser/android/app_web_message_port_service_impl.h>
remove?

https://codereview.chromium.org/2375133002/diff/1/android_webview/native/andr...
File android_webview/native/android_webview_jni_registrar.cc (right):

https://codereview.chromium.org/2375133002/diff/1/android_webview/native/andr...
android_webview/native/android_webview_jni_registrar.cc:5: #include
<content/browser/android/app_web_message_port_service_impl.h>
remove

https://codereview.chromium.org/2375133002/diff/1/android_webview/native/aw_c...
File android_webview/native/aw_contents.cc (right):

https://codereview.chromium.org/2375133002/diff/1/android_webview/native/aw_c...
android_webview/native/aw_contents.cc:1233:
&content::AppWebMessagePortService::RemoveSentPorts,
as webmessageportservice is now in content, you should be able to move this
piece to the implementation of messageportprovider::postmessagetoframe.

this would also let you simplify public appwebmessageportservice interface.

https://codereview.chromium.org/2375133002/diff/1/android_webview/native/aw_c...
File android_webview/native/aw_contents.h (right):

https://codereview.chromium.org/2375133002/diff/1/android_webview/native/aw_c...
android_webview/native/aw_contents.h:36: class MessagePortDelegate;
remove

https://codereview.chromium.org/2375133002/diff/1/android_webview/renderer/aw...
File android_webview/renderer/aw_content_renderer_client.cc (right):

https://codereview.chromium.org/2375133002/diff/1/android_webview/renderer/aw...
android_webview/renderer/aw_content_renderer_client.cc:5: #include
<content/renderer/android/app_web_message_port_client.h>
remove

https://codereview.chromium.org/2375133002/diff/1/content/browser/android/app...
File content/browser/android/app_web_message_port_message_filter.cc (right):

https://codereview.chromium.org/2375133002/diff/1/content/browser/android/app...
content/browser/android/app_web_message_port_message_filter.cc:5: #include
"content/browser/android/app_web_message_port_message_filter.h"
can you lower the similarity parameter so rietveld recognizes the move, making
the review easier.

https://codereview.chromium.org/2375133002/diff/1/content/browser/android/app...
content/browser/android/app_web_message_port_message_filter.cc:37:
AppWebMessagePortService::OnConvertedWebToAppMessage)
directly use the impl class

https://codereview.chromium.org/2375133002/diff/1/content/browser/android/app...
File content/browser/android/app_web_message_port_service_impl.h (right):

https://codereview.chromium.org/2375133002/diff/1/content/browser/android/app...
content/browser/android/app_web_message_port_service_impl.h:23: 
do the notes in java class still apply after this move?

https://codereview.chromium.org/2375133002/diff/1/content/browser/android/app...
content/browser/android/app_web_message_port_service_impl.h:46: void
RemoveSentPorts(const std::vector<int>& sent_ports) override;
private

https://codereview.chromium.org/2375133002/diff/1/content/browser/message_por...
File content/browser/message_port_provider.cc (right):

https://codereview.chromium.org/2375133002/diff/1/content/browser/message_por...
content/browser/message_port_provider.cc:16: #if defined(OS_ANDROID)
this sounds like the code is not well spread between android/non-android code.
Should message_port_provider move to android specific directories?

Actually I think you can simply move this to appwebmessageportserviceimpl?

https://codereview.chromium.org/2375133002/diff/1/content/common/app_web_mess...
File content/common/app_web_message_port_messages.h (right):

https://codereview.chromium.org/2375133002/diff/1/content/common/app_web_mess...
content/common/app_web_message_port_messages.h:28: // that browser can use.
Since WebView is single-process this is not terribly
this statement feels like needs clarification as chrome is multiprocess.

https://codereview.chromium.org/2375133002/diff/1/content/common/app_web_mess...
content/common/app_web_message_port_messages.h:32: // Important Note about
multi-process situation: Webview is single process so
this badly feels more context: after moving to CCT, why/how is the security
situation fixed in CCT or is this not an issue actually?

https://codereview.chromium.org/2375133002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePort.java
(right):

https://codereview.chromium.org/2375133002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePort.java:63:
* Webview puts a port to a "started" state if:
please make the documentation agnostic to webview since now the code is under
content.

https://codereview.chromium.org/2375133002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePortService.java
(right):

https://codereview.chromium.org/2375133002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePortService.java:15:
* Provides the Message Channel functionality for Android Webview. Specifically
webview in documentation again

https://codereview.chromium.org/2375133002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePortService.java:27:
* Ownership: The Java AppWebMessagePortService is owned by Java
AwBrowserContext.
update ownership

https://codereview.chromium.org/2375133002/diff/1/content/public/browser/andr...
File content/public/browser/android/app_web_message_port_service.h (right):

https://codereview.chromium.org/2375133002/diff/1/content/public/browser/andr...
content/public/browser/android/app_web_message_port_service.h:24: virtual void
OnConvertedWebToAppMessage(
no need to be in public interface?

https://codereview.chromium.org/2375133002/diff/1/content/public/browser/andr...
content/public/browser/android/app_web_message_port_service.h:29: virtual void
CleanupPort(int message_port_id) = 0;
I don't think you need this in public interface anymore

https://codereview.chromium.org/2375133002/diff/1/content/public/browser/mess...
File content/public/browser/message_port_provider.h (right):

https://codereview.chromium.org/2375133002/diff/1/content/public/browser/mess...
content/public/browser/message_port_provider.h:22: class CONTENT_EXPORT
MessagePortProvider {
I think you can move postmessagetoframe to appwebmessageportservice, together
with the getter method. then you can get rid of this class completely.

There are tests that directly refer to MessagePortProvider, so they also need to
move then.

[Yusuf, 2016-10-04 21:33:15]

https://codereview.chromium.org/2375133002/diff/1/android_webview/lib/main/aw...
File android_webview/lib/main/aw_main_delegate.cc (right):

https://codereview.chromium.org/2375133002/diff/1/android_webview/lib/main/aw...
android_webview/lib/main/aw_main_delegate.cc:5: #include
<content/browser/android/app_web_message_port_service_impl.h>
On 2016/09/30 23:40:59, sgurun wrote:
> remove?

Done.

https://codereview.chromium.org/2375133002/diff/1/android_webview/native/andr...
File android_webview/native/android_webview_jni_registrar.cc (right):

https://codereview.chromium.org/2375133002/diff/1/android_webview/native/andr...
android_webview/native/android_webview_jni_registrar.cc:5: #include
<content/browser/android/app_web_message_port_service_impl.h>
On 2016/09/30 23:40:59, sgurun wrote:
> remove

Done.

https://codereview.chromium.org/2375133002/diff/1/android_webview/native/aw_c...
File android_webview/native/aw_contents.cc (right):

https://codereview.chromium.org/2375133002/diff/1/android_webview/native/aw_c...
android_webview/native/aw_contents.cc:1233:
&content::AppWebMessagePortService::RemoveSentPorts,
On 2016/09/30 23:40:59, sgurun wrote:
> as webmessageportservice is now in content, you should be able to move this
> piece to the implementation of messageportprovider::postmessagetoframe.
> 
> this would also let you simplify public appwebmessageportservice interface.

Done.

https://codereview.chromium.org/2375133002/diff/1/android_webview/native/aw_c...
File android_webview/native/aw_contents.h (right):

https://codereview.chromium.org/2375133002/diff/1/android_webview/native/aw_c...
android_webview/native/aw_contents.h:36: class MessagePortDelegate;
On 2016/09/30 23:40:59, sgurun wrote:
> remove

Done.

https://codereview.chromium.org/2375133002/diff/1/android_webview/renderer/aw...
File android_webview/renderer/aw_content_renderer_client.cc (right):

https://codereview.chromium.org/2375133002/diff/1/android_webview/renderer/aw...
android_webview/renderer/aw_content_renderer_client.cc:5: #include
<content/renderer/android/app_web_message_port_client.h>
On 2016/09/30 23:40:59, sgurun wrote:
> remove

Done.

https://codereview.chromium.org/2375133002/diff/1/content/browser/android/app...
File content/browser/android/app_web_message_port_message_filter.cc (right):

https://codereview.chromium.org/2375133002/diff/1/content/browser/android/app...
content/browser/android/app_web_message_port_message_filter.cc:5: #include
"content/browser/android/app_web_message_port_message_filter.h"
On 2016/09/30 23:40:59, sgurun wrote:
> can you lower the similarity parameter so rietveld recognizes the move, making
> the review easier.

Umm, sorry I am not sure I understand how I do that. A bit more detail on this
one?

https://codereview.chromium.org/2375133002/diff/1/content/browser/android/app...
content/browser/android/app_web_message_port_message_filter.cc:37:
AppWebMessagePortService::OnConvertedWebToAppMessage)
On 2016/09/30 23:40:59, sgurun wrote:
> directly use the impl class

Done.

https://codereview.chromium.org/2375133002/diff/1/content/browser/android/app...
File content/browser/android/app_web_message_port_service_impl.h (right):

https://codereview.chromium.org/2375133002/diff/1/content/browser/android/app...
content/browser/android/app_web_message_port_service_impl.h:23: 
On 2016/09/30 23:40:59, sgurun wrote:
> do the notes in java class still apply after this move?

Yes!

https://codereview.chromium.org/2375133002/diff/1/content/browser/android/app...
content/browser/android/app_web_message_port_service_impl.h:46: void
RemoveSentPorts(const std::vector<int>& sent_ports) override;
On 2016/09/30 23:40:59, sgurun wrote:
> private

Got rid of override, but had to keep it public since MessagePortProvider calls
it in the impl class still.

https://codereview.chromium.org/2375133002/diff/1/content/browser/message_por...
File content/browser/message_port_provider.cc (right):

https://codereview.chromium.org/2375133002/diff/1/content/browser/message_por...
content/browser/message_port_provider.cc:16: #if defined(OS_ANDROID)
On 2016/09/30 23:40:59, sgurun wrote:
> this sounds like the code is not well spread between android/non-android code.
> Should message_port_provider move to android specific directories?
> 
> Actually I think you can simply move this to appwebmessageportserviceimpl?

We still need the PostMessageToFrame since it is used by aw_contents and
web_contents_impl to directly send messages without using message ports.

But moving it to an Android specific folder makes sense.

https://codereview.chromium.org/2375133002/diff/1/content/common/app_web_mess...
File content/common/app_web_message_port_messages.h (right):

https://codereview.chromium.org/2375133002/diff/1/content/common/app_web_mess...
content/common/app_web_message_port_messages.h:28: // that browser can use.
Since WebView is single-process this is not terribly
On 2016/09/30 23:40:59, sgurun wrote:
> this statement feels like needs clarification as chrome is multiprocess.

Done.

https://codereview.chromium.org/2375133002/diff/1/content/common/app_web_mess...
content/common/app_web_message_port_messages.h:32: // Important Note about
multi-process situation: Webview is single process so
On 2016/09/30 23:40:59, sgurun wrote:
> this badly feels more context: after moving to CCT, why/how is the security
> situation fixed in CCT or is this not an issue actually?

I tried to reinterpret the issue through rephrasing. With my admittedly limited
knowledge of the multi process implications of this, I don't think we have an
issue in Chrome land as well. (Things might get worrying when we conside out of
process iframes etc, I think).

I am open to both technical and comments improvements suggestions on this one.

https://codereview.chromium.org/2375133002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePort.java
(right):

https://codereview.chromium.org/2375133002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePort.java:63:
* Webview puts a port to a "started" state if:
On 2016/09/30 23:40:59, sgurun wrote:
> please make the documentation agnostic to webview since now the code is under
> content. 

Done.

https://codereview.chromium.org/2375133002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePortService.java
(right):

https://codereview.chromium.org/2375133002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePortService.java:15:
* Provides the Message Channel functionality for Android Webview. Specifically
On 2016/09/30 23:40:59, sgurun wrote:
> webview in documentation again

Done.

https://codereview.chromium.org/2375133002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePortService.java:27:
* Ownership: The Java AppWebMessagePortService is owned by Java
AwBrowserContext.
On 2016/09/30 23:40:59, sgurun wrote:
> update ownership

Done.

https://codereview.chromium.org/2375133002/diff/1/content/public/browser/andr...
File content/public/browser/android/app_web_message_port_service.h (right):

https://codereview.chromium.org/2375133002/diff/1/content/public/browser/andr...
content/public/browser/android/app_web_message_port_service.h:24: virtual void
OnConvertedWebToAppMessage(
On 2016/09/30 23:40:59, sgurun wrote:
> no need to be in public interface?

Done.

https://codereview.chromium.org/2375133002/diff/1/content/public/browser/andr...
content/public/browser/android/app_web_message_port_service.h:29: virtual void
CleanupPort(int message_port_id) = 0;
On 2016/09/30 23:40:59, sgurun wrote:
> I don't think you need this in public interface anymore

Done.

https://codereview.chromium.org/2375133002/diff/1/content/public/browser/mess...
File content/public/browser/message_port_provider.h (right):

https://codereview.chromium.org/2375133002/diff/1/content/public/browser/mess...
content/public/browser/message_port_provider.h:22: class CONTENT_EXPORT
MessagePortProvider {
On 2016/09/30 23:40:59, sgurun wrote:
> I think you can move postmessagetoframe to appwebmessageportservice, together
> with the getter method. then you can get rid of this class completely.
> 
> There are tests that directly refer to MessagePortProvider, so they also need
to
> move then.

Sadly we still have the aw_contents and web_contents_impl sending messages to
main frame without any ports use case. That is why we need a content side getter
for the impl.

[sgurun-gerrit only, 2016-10-06 22:57:39]

https://codereview.chromium.org/2375133002/diff/60001/android_webview/browser...
File android_webview/browser/jni_dependency_factory.h (right):

https://codereview.chromium.org/2375133002/diff/60001/android_webview/browser...
android_webview/browser/jni_dependency_factory.h:18: class
AppWebMessagePortService;
remove

https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
File content/browser/android/app_web_message_port_message_filter.cc (right):

https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
content/browser/android/app_web_message_port_message_filter.cc:51:
msp->PostMessage(msg_port_id, message, sent_message_port_ids);
nit: alignment. did you run git cl format?

https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
File content/browser/android/app_web_message_port_service_impl.cc (right):

https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
content/browser/android/app_web_message_port_service_impl.cc:55:
AppWebMessagePortMessageFilter* filter = new AppWebMessagePortMessageFilter(
I think this is wrong.

The logic in aw_contents keep an instance variable to store the filter and
returns it if it already exists. This one creates a new one each time
createmessagechannel is called.

this filter is used to convert messages between formats. You should not need to
create a new one for each.

https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
content/browser/android/app_web_message_port_service_impl.cc:225:
MessagePortService::GetInstance()->HoldMessages(*portId1);
use msp here and below.

https://codereview.chromium.org/2375133002/diff/60001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePortService.java
(right):

https://codereview.chromium.org/2375133002/diff/60001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePortService.java:15:
* Provides the Message Channel functionality for Android. Specifically
Let's at least mention once that by saying Android, we mean Android Apps
(including WebView embedders and CCT users) here.

The HTML5 message channel implementation (for android or in general) is not
implemented here and is not changed in any way.

[Yusuf, 2016-10-07 21:20:28]

https://codereview.chromium.org/2375133002/diff/60001/android_webview/browser...
File android_webview/browser/jni_dependency_factory.h (right):

https://codereview.chromium.org/2375133002/diff/60001/android_webview/browser...
android_webview/browser/jni_dependency_factory.h:18: class
AppWebMessagePortService;
On 2016/10/06 22:57:39, sgurun wrote:
> remove

Done.

https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
File content/browser/android/app_web_message_port_message_filter.cc (right):

https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
content/browser/android/app_web_message_port_message_filter.cc:51:
msp->PostMessage(msg_port_id, message, sent_message_port_ids);
On 2016/10/06 22:57:39, sgurun wrote:
> nit: alignment. did you run git cl format?

Done.

https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
File content/browser/android/app_web_message_port_service_impl.cc (right):

https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
content/browser/android/app_web_message_port_service_impl.cc:55:
AppWebMessagePortMessageFilter* filter = new AppWebMessagePortMessageFilter(
On 2016/10/06 22:57:39, sgurun wrote:
> I think this is wrong.
> 
> The logic in aw_contents keep an instance variable to store the filter and
> returns it if it already exists. This one creates a new one each time
> createmessagechannel is called.
> 
> this filter is used to convert messages between formats. You should not need
to
> create a new one for each.
> 

Oops, you are right.

Moved the ownership to render_frame_host_impl.

jochen@, I know there might be a bit too many ifdefs there, any suggestions to
how to avoid them while addressing the ownership issue would be appreciated.

https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
content/browser/android/app_web_message_port_service_impl.cc:225:
MessagePortService::GetInstance()->HoldMessages(*portId1);
On 2016/10/06 22:57:39, sgurun wrote:
> use msp here and below.

Done.

https://codereview.chromium.org/2375133002/diff/60001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePortService.java
(right):

https://codereview.chromium.org/2375133002/diff/60001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePortService.java:15:
* Provides the Message Channel functionality for Android. Specifically
On 2016/10/06 22:57:39, sgurun wrote:
> Let's at least mention once that by saying Android, we mean Android Apps
> (including WebView embedders and CCT users) here.
> 
> The HTML5 message channel implementation (for android or in general) is not
> implemented here and is not changed in any way.

Done.

[sgurun-gerrit only, 2016-10-07 22:13:20]

On 2016/10/07 21:20:28, Yusuf wrote:
>
https://codereview.chromium.org/2375133002/diff/60001/android_webview/browser...
> File android_webview/browser/jni_dependency_factory.h (right):
> 
>
https://codereview.chromium.org/2375133002/diff/60001/android_webview/browser...
> android_webview/browser/jni_dependency_factory.h:18: class
> AppWebMessagePortService;
> On 2016/10/06 22:57:39, sgurun wrote:
> > remove
> 
> Done.
> 
>
https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
> File content/browser/android/app_web_message_port_message_filter.cc (right):
> 
>
https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
> content/browser/android/app_web_message_port_message_filter.cc:51:
> msp->PostMessage(msg_port_id, message, sent_message_port_ids);
> On 2016/10/06 22:57:39, sgurun wrote:
> > nit: alignment. did you run git cl format?
> 
> Done.
> 
>
https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
> File content/browser/android/app_web_message_port_service_impl.cc (right):
> 
>
https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
> content/browser/android/app_web_message_port_service_impl.cc:55:
> AppWebMessagePortMessageFilter* filter = new AppWebMessagePortMessageFilter(
> On 2016/10/06 22:57:39, sgurun wrote:
> > I think this is wrong.
> > 
> > The logic in aw_contents keep an instance variable to store the filter and
> > returns it if it already exists. This one creates a new one each time
> > createmessagechannel is called.
> > 
> > this filter is used to convert messages between formats. You should not need
> to
> > create a new one for each.
> > 
> 
> Oops, you are right.
> 
> Moved the ownership to render_frame_host_impl.
> 
> jochen@, I know there might be a bit too many ifdefs there, any suggestions to
> how to avoid them while addressing the ownership issue would be appreciated.
> 
>
https://codereview.chromium.org/2375133002/diff/60001/content/browser/android...
> content/browser/android/app_web_message_port_service_impl.cc:225:
> MessagePortService::GetInstance()->HoldMessages(*portId1);
> On 2016/10/06 22:57:39, sgurun wrote:
> > use msp here and below.
> 
> Done.
> 
>
https://codereview.chromium.org/2375133002/diff/60001/content/public/android/...
> File
>
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePortService.java
> (right):
> 
>
https://codereview.chromium.org/2375133002/diff/60001/content/public/android/...
>
content/public/android/java/src/org/chromium/content/browser/AppWebMessagePortService.java:15:
> * Provides the Message Channel functionality for Android. Specifically
> On 2016/10/06 22:57:39, sgurun wrote:
> > Let's at least mention once that by saying Android, we mean Android Apps
> > (including WebView embedders and CCT users) here.
> > 
> > The HTML5 message channel implementation (for android or in general) is not
> > implemented here and is not changed in any way.
> 
> Done.

lgtm with my comment

[sgurun-gerrit only, 2016-10-07 22:13:31]

https://codereview.chromium.org/2375133002/diff/80001/content/public/browser/...
File content/public/browser/message_port_provider.h (right):

https://codereview.chromium.org/2375133002/diff/80001/content/public/browser/...
content/public/browser/message_port_provider.h:37: static
content::AppWebMessagePortService* GetAppWebMessagePortService();
should this be ifdef ANDROID since it only works for Android?

[Yusuf, 2016-10-08 02:43:04]

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

[commit-bot: I haz the power, 2016-10-08 02:43:12]

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, 2016-10-08 04:08:14]

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

[commit-bot: I haz the power, 2016-10-08 04:08:15]

Dry run: Exceeded global retry quota

[jochen (gone - plz use gerrit), 2016-10-10 12:40:07]

lgtm

[Yusuf, 2016-10-10 20:19:20]

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

[commit-bot: I haz the power, 2016-10-10 20:19:47]

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, 2016-10-10 21:24:11]

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

[commit-bot: I haz the power, 2016-10-10 21:24:12]

Dry run: This issue passed the CQ dry run.

[Yusuf, 2016-10-10 22:01:06]

The CQ bit was checked by yusufo@chromium.org

[Yusuf, 2016-10-10 22:01:07]

The patchset sent to the CQ was uploaded after l-g-t-m from sgurun@chromium.org,
jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2375133002/#ps120001
(title: "added ifdef")

[commit-bot: I haz the power, 2016-10-10 22:01:44]

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, 2016-10-10 22:12:23]

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

[commit-bot: I haz the power, 2016-10-10 22:12:25]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Yusuf, 2016-10-10 22:35:38]

yusufo@chromium.org changed reviewers:
+ rsesek@chromium.org

[Yusuf, 2016-10-10 22:35:39]

rsesek@ for _messages.h file move

rsesek@ this CL is for moving the postMessage API that android_webview was using
to content, so that it can also be used for custom tabs.

The move involves moving the ipc messages as well.

[Robert Sesek, 2016-10-11 15:44:34]

Just some nits.

https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
File content/browser/android/app_web_message_port_message_filter.cc (right):

https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
content/browser/android/app_web_message_port_message_filter.cc:5: #include
"content/browser/android/app_web_message_port_message_filter.h"
nit: blank line after

https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
File content/browser/android/app_web_message_port_message_filter.h (right):

https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
content/browser/android/app_web_message_port_message_filter.h:18: public
content::MessagePortDelegate {
nit: align with 'p' above

https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
File content/browser/android/app_web_message_port_service_impl.cc (right):

https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
content/browser/android/app_web_message_port_service_impl.cc:112: jobj,
nit: alignment

https://codereview.chromium.org/2375133002/diff/120001/content/browser/frame_...
File content/browser/frame_host/render_frame_host_impl.h (right):

https://codereview.chromium.org/2375133002/diff/120001/content/browser/frame_...
content/browser/frame_host/render_frame_host_impl.h:1071: 
extra blank line

https://codereview.chromium.org/2375133002/diff/120001/content/renderer/andro...
File content/renderer/android/app_web_message_port_client.cc (right):

https://codereview.chromium.org/2375133002/diff/120001/content/renderer/andro...
content/renderer/android/app_web_message_port_client.cc:5: #include
<content/common/app_web_message_port_messages.h>
Wrong #include type for local paths. Should be "".

[sgurun-gerrit only, 2016-10-11 16:00:43]

On 2016/10/11 15:44:34, Robert Sesek wrote:
> Just some nits.
> 
>
https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
> File content/browser/android/app_web_message_port_message_filter.cc (right):
> 
>
https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
> content/browser/android/app_web_message_port_message_filter.cc:5: #include
> "content/browser/android/app_web_message_port_message_filter.h"
> nit: blank line after
> 
>
https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
> File content/browser/android/app_web_message_port_message_filter.h (right):
> 
>
https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
> content/browser/android/app_web_message_port_message_filter.h:18: public
> content::MessagePortDelegate {
> nit: align with 'p' above
> 
>
https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
> File content/browser/android/app_web_message_port_service_impl.cc (right):
> 
>
https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
> content/browser/android/app_web_message_port_service_impl.cc:112: jobj,
> nit: alignment
> 
>
https://codereview.chromium.org/2375133002/diff/120001/content/browser/frame_...
> File content/browser/frame_host/render_frame_host_impl.h (right):
> 
>
https://codereview.chromium.org/2375133002/diff/120001/content/browser/frame_...
> content/browser/frame_host/render_frame_host_impl.h:1071: 
> extra blank line
> 
>
https://codereview.chromium.org/2375133002/diff/120001/content/renderer/andro...
> File content/renderer/android/app_web_message_port_client.cc (right):
> 
>
https://codereview.chromium.org/2375133002/diff/120001/content/renderer/andro...
> content/renderer/android/app_web_message_port_client.cc:5: #include
> <content/common/app_web_message_port_messages.h>
> Wrong #include type for local paths. Should be "".

would not git cl format catch these? or if you did not run it, I highly
recommend to run it.

[Yusuf, 2016-10-11 16:48:47]

And also ran "git cl format" per sgurun's suggestion. Like you said all except
the include "" vs <> were caught and some others as well.

https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
File content/browser/android/app_web_message_port_message_filter.cc (right):

https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
content/browser/android/app_web_message_port_message_filter.cc:5: #include
"content/browser/android/app_web_message_port_message_filter.h"
On 2016/10/11 15:44:33, Robert Sesek wrote:
> nit: blank line after

Done.

https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
File content/browser/android/app_web_message_port_message_filter.h (right):

https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
content/browser/android/app_web_message_port_message_filter.h:18: public
content::MessagePortDelegate {
On 2016/10/11 15:44:33, Robert Sesek wrote:
> nit: align with 'p' above

Done.

https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
File content/browser/android/app_web_message_port_service_impl.cc (right):

https://codereview.chromium.org/2375133002/diff/120001/content/browser/androi...
content/browser/android/app_web_message_port_service_impl.cc:112: jobj,
On 2016/10/11 15:44:33, Robert Sesek wrote:
> nit: alignment

Done.

https://codereview.chromium.org/2375133002/diff/120001/content/browser/frame_...
File content/browser/frame_host/render_frame_host_impl.h (right):

https://codereview.chromium.org/2375133002/diff/120001/content/browser/frame_...
content/browser/frame_host/render_frame_host_impl.h:1071: 
On 2016/10/11 15:44:33, Robert Sesek wrote:
> extra blank line

Done.

https://codereview.chromium.org/2375133002/diff/120001/content/renderer/andro...
File content/renderer/android/app_web_message_port_client.cc (right):

https://codereview.chromium.org/2375133002/diff/120001/content/renderer/andro...
content/renderer/android/app_web_message_port_client.cc:5: #include
<content/common/app_web_message_port_messages.h>
On 2016/10/11 15:44:34, Robert Sesek wrote:
> Wrong #include type for local paths. Should be "".

Done.

[Robert Sesek, 2016-10-11 16:54:34]

LGTM

[Yusuf, 2016-10-11 17:00:35]

The CQ bit was checked by yusufo@chromium.org

[Yusuf, 2016-10-11 17:00:35]

The patchset sent to the CQ was uploaded after l-g-t-m from sgurun@chromium.org,
jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2375133002/#ps140001
(title: "rsesek nits and git cl format")

[commit-bot: I haz the power, 2016-10-11 17:00:48]

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, 2016-10-11 17:10:48]

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

[commit-bot: I haz the power, 2016-10-11 17:10:50]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[Yusuf, 2016-10-11 18:59:21]

The CQ bit was checked by yusufo@chromium.org

[commit-bot: I haz the power, 2016-10-11 18:59:49]

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, 2016-10-11 19:10:01]

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

[commit-bot: I haz the power, 2016-10-11 19:10:03]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Yusuf, 2016-10-11 20:38:37]

The CQ bit was checked by yusufo@chromium.org

[commit-bot: I haz the power, 2016-10-11 20:39:14]

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, 2016-10-11 20:59:34]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-10-11 21:03:39]

Description was changed from

==========
Move MessagePort implementation from android_webview to content

-Rename all classes with AppWeb prefix instead of Aw
-Move browser side classes to content/browser/android
-Move renderer side class to content/renderer/android
-Add public interface in content for app_web_message_port_service
-Slim down message_port_provider now that all classes are in content
-Move all java classes to content/public/android with package
org.chromium.content.browser
-AppWebMessagePortMessageFilter ownership moves to RenderFrameHostImpl
-AppWebMessagePortClient ownership moves to RenderFrameImpl

BUG=651124
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Move MessagePort implementation from android_webview to content

-Rename all classes with AppWeb prefix instead of Aw
-Move browser side classes to content/browser/android
-Move renderer side class to content/renderer/android
-Add public interface in content for app_web_message_port_service
-Slim down message_port_provider now that all classes are in content
-Move all java classes to content/public/android with package
org.chromium.content.browser
-AppWebMessagePortMessageFilter ownership moves to RenderFrameHostImpl
-AppWebMessagePortClient ownership moves to RenderFrameImpl

BUG=651124
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/ed66fc0d1f1d7adb27862c6046212162445231ac
Cr-Commit-Position: refs/heads/master@{#424543}
==========

[commit-bot: I haz the power, 2016-10-11 21:03:41]

Patchset 8 (id:??) landed as
https://crrev.com/ed66fc0d1f1d7adb27862c6046212162445231ac
Cr-Commit-Position: refs/heads/master@{#424543}