Introduce a secondary map for enabling plznavigate

Introduce a secondary map 

Once browser initiated nagivations are enabled, we need a secondary map to map framenodeid's to IO thread clients. 

BUG=645983
Review-Url: https://codereview.chromium.org/2737663004
Cr-Commit-Position: refs/heads/master@{#456126}
Committed: https://chromium.googlesource.com/chromium/src/+/ea1c1679e17de2aebbdbeab39149b1890914b260

[sgurun-gerrit only, 2017-03-09 20:33:34]

Description was changed from

==========
[DRAFT ] Introduce a secondary map and enable plznavigate

BUG=
==========

to

==========
Introduce a secondary map 

Once browser initiated nagivations are enabled, we need a secondary map to map
framenodeid's to IO thread clients. 

BUG=645983
==========

[sgurun-gerrit only, 2017-03-09 20:33:34]

sgurun@chromium.org changed reviewers:
+ jam@chromium.org

[sgurun-gerrit only, 2017-03-09 20:35:58]

On 2017/03/09 20:33:34, sgurun wrote:
> mailto:sgurun@chromium.org changed reviewers:
> + mailto:jam@chromium.org

there is a little bit of more massaging needed such as perhaps combining FromID.
And AwCookieAccessPolicy.

[jam, 2017-03-09 22:13:05]

https://codereview.chromium.org/2737663004/diff/120001/android_webview/browse...
File android_webview/browser/aw_contents_io_thread_client.h (right):

https://codereview.chromium.org/2737663004/diff/120001/android_webview/browse...
android_webview/browser/aw_contents_io_thread_client.h:66: static
std::unique_ptr<AwContentsIoThreadClient> FromID(
nit: document, i.e. so someone reading this knows why there are two methods

https://codereview.chromium.org/2737663004/diff/120001/android_webview/browse...
File android_webview/browser/net/aw_request_interceptor.cc (right):

https://codereview.chromium.org/2737663004/diff/120001/android_webview/browse...
android_webview/browser/net/aw_request_interceptor.cc:129: if (render_process_id
== -1 || render_frame_id == -1) {
since this block (and rest of method) is duplicated in
aw_resource_dispatcher_host_delegate.cc, can this be moved into a static method
in AwContentsIoThreadClient? Even better if we can hide the existing two that
take in RPH/RFH and force people to pass in a net::URLRequest or
content::ResourceRequestInfo; that way we ensure that we won't gain new
callsites in the future that fail for navigation requests

https://codereview.chromium.org/2737663004/diff/120001/android_webview/browse...
android_webview/browser/net/aw_request_interceptor.cc:157: if
(request->url().SchemeIs(url::kBlobScheme)) {
comment?

https://codereview.chromium.org/2737663004/diff/120001/android_webview/native...
File android_webview/native/aw_contents_io_thread_client_impl.cc (right):

https://codereview.chromium.org/2737663004/diff/120001/android_webview/native...
android_webview/native/aw_contents_io_thread_client_impl.cc:60: // Needed for
PlzNavigate.
nit: expand on the comment to explain why navigation requests don't have RPH/RFH
IDs and instead we have to use the FTN ID

[jam, 2017-03-10 17:08:44]

https://codereview.chromium.org/2737663004/diff/120001/android_webview/browse...
File android_webview/browser/net/aw_request_interceptor.cc (right):

https://codereview.chromium.org/2737663004/diff/120001/android_webview/browse...
android_webview/browser/net/aw_request_interceptor.cc:129: if (render_process_id
== -1 || render_frame_id == -1) {
On 2017/03/09 22:13:05, jam wrote:
> since this block (and rest of method) is duplicated in
> aw_resource_dispatcher_host_delegate.cc, can this be moved into a static
method
> in AwContentsIoThreadClient? Even better if we can hide the existing two that
> take in RPH/RFH and force people to pass in a net::URLRequest or
> content::ResourceRequestInfo; that way we ensure that we won't gain new
> callsites in the future that fail for navigation requests

Ok ignore this comment, I looked through the usage of
AwContentsIoThreadClient::FromID and the cookie one has to send RPH/RFH IDs
since it's in response to the renderer which doesn't have a net::URLRequest (and
it's fine with PlzNavigate).

[jam, 2017-03-10 17:08:58]

lgtm

[sgurun-gerrit only, 2017-03-10 18:40:58]

https://codereview.chromium.org/2737663004/diff/120001/android_webview/browse...
File android_webview/browser/aw_contents_io_thread_client.h (right):

https://codereview.chromium.org/2737663004/diff/120001/android_webview/browse...
android_webview/browser/aw_contents_io_thread_client.h:66: static
std::unique_ptr<AwContentsIoThreadClient> FromID(
On 2017/03/09 22:13:05, jam wrote:
> nit: document, i.e. so someone reading this knows why there are two methods

Done.

https://codereview.chromium.org/2737663004/diff/120001/android_webview/browse...
File android_webview/browser/net/aw_request_interceptor.cc (right):

https://codereview.chromium.org/2737663004/diff/120001/android_webview/browse...
android_webview/browser/net/aw_request_interceptor.cc:129: if (render_process_id
== -1 || render_frame_id == -1) {
On 2017/03/10 17:08:44, jam wrote:
> On 2017/03/09 22:13:05, jam wrote:
> > since this block (and rest of method) is duplicated in
> > aw_resource_dispatcher_host_delegate.cc, can this be moved into a static
> method
> > in AwContentsIoThreadClient? Even better if we can hide the existing two
that
> > take in RPH/RFH and force people to pass in a net::URLRequest or
> > content::ResourceRequestInfo; that way we ensure that we won't gain new
> > callsites in the future that fail for navigation requests
> 
> Ok ignore this comment, I looked through the usage of
> AwContentsIoThreadClient::FromID and the cookie one has to send RPH/RFH IDs
> since it's in response to the renderer which doesn't have a net::URLRequest
(and
> it's fine with PlzNavigate).

Acknowledged.

https://codereview.chromium.org/2737663004/diff/120001/android_webview/browse...
android_webview/browser/net/aw_request_interceptor.cc:157: if
(request->url().SchemeIs(url::kBlobScheme)) {
On 2017/03/09 22:13:05, jam wrote:
> comment?

Done.

https://codereview.chromium.org/2737663004/diff/120001/android_webview/native...
File android_webview/native/aw_contents_io_thread_client_impl.cc (right):

https://codereview.chromium.org/2737663004/diff/120001/android_webview/native...
android_webview/native/aw_contents_io_thread_client_impl.cc:60: // Needed for
PlzNavigate.
On 2017/03/09 22:13:05, jam wrote:
> nit: expand on the comment to explain why navigation requests don't have
RPH/RFH
> IDs and instead we have to use the FTN ID

Done.

[sgurun-gerrit only, 2017-03-10 18:42:15]

The CQ bit was checked by sgurun@chromium.org

[sgurun-gerrit only, 2017-03-10 18:42:15]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2737663004/#ps140001
(title: "address jam review")

[commit-bot: I haz the power, 2017-03-10 18:42:38]

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-03-10 19:14:40]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1489171335581180,
"parent_rev": "1bbc9fc2b43ec7b02f5f781ffb70675c5def4220", "commit_rev":
"ea1c1679e17de2aebbdbeab39149b1890914b260"}

[commit-bot: I haz the power, 2017-03-10 19:15:19]

Description was changed from

==========
Introduce a secondary map 

Once browser initiated nagivations are enabled, we need a secondary map to map
framenodeid's to IO thread clients. 

BUG=645983
==========

to

==========
Introduce a secondary map 

Once browser initiated nagivations are enabled, we need a secondary map to map
framenodeid's to IO thread clients. 

BUG=645983

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

[commit-bot: I haz the power, 2017-03-10 19:15:20]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/ea1c1679e17de2aebbdbeab39149...