PlzNavigate support in data use ascriber

PlzNavigate support in data use ascriber

With PlzNavigate, RenderFrameHost will not be populated in mainframe
requests. So global request ID alone should be used for mapping those
requests to main renderframe, when navigation commits later. This CL is 
based off of Chromium CL 2874353002.

BUG=664233
TBR=rajendrant@chromium.org

Review-Url: https://codereview.chromium.org/2913063002
Cr-Commit-Position: refs/heads/master@{#476006}
Committed: https://chromium.googlesource.com/chromium/src/+/975ba1a456087d8781f61754b747c66dab30c97f

[tbansal1, 2017-05-30 21:14:52]

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

[commit-bot: I haz the power, 2017-05-30 21:15:27]

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-30 21:57:10]

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

[commit-bot: I haz the power, 2017-05-30 21:57:11]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tbansal1, 2017-05-30 23:08:02]

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

[commit-bot: I haz the power, 2017-05-30 23:08:48]

Dry run: CQ is trying da patch.

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

[tbansal1, 2017-05-30 23:38:59]

Patchset #1 (id:1) has been deleted

[tbansal1, 2017-05-30 23:39:28]

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

[commit-bot: I haz the power, 2017-05-30 23:40:24]

Dry run: CQ is trying da patch.

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

[tbansal1, 2017-05-30 23:43:26]

Description was changed from

==========
ascriber: plz navigate
==========

to

==========
PlzNavigate support in data use ascriber

With PlzNavigate, RenderFrameHost will not be populated in mainframe
requests. So global request ID alone should be used for mapping those
requests to main renderframe, when navigation commits later. This CL is based
off of Chromium CL 2874353002.

BUG=664233
==========

[tbansal1, 2017-05-30 23:43:36]

Description was changed from

==========
PlzNavigate support in data use ascriber

With PlzNavigate, RenderFrameHost will not be populated in mainframe
requests. So global request ID alone should be used for mapping those
requests to main renderframe, when navigation commits later. This CL is based
off of Chromium CL 2874353002.

BUG=664233
==========

to

==========
PlzNavigate support in data use ascriber

With PlzNavigate, RenderFrameHost will not be populated in mainframe
requests. So global request ID alone should be used for mapping those
requests to main renderframe, when navigation commits later. This CL is 
based off of Chromium CL 2874353002.

BUG=664233
==========

[commit-bot: I haz the power, 2017-05-31 00:28:53]

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

[commit-bot: I haz the power, 2017-05-31 00:28:54]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tbansal1, 2017-05-31 06:35:06]

Patchset #1 (id:20001) has been deleted

[tbansal1, 2017-05-31 06:36:45]

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

[commit-bot: I haz the power, 2017-05-31 06:37:03]

Dry run: CQ is trying da patch.

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

[tbansal1, 2017-05-31 06:46:58]

Patchset #3 (id:80001) has been deleted

[tbansal1, 2017-05-31 06:52:02]

Patchset #3 (id:100001) has been deleted

[tbansal1, 2017-05-31 06:52:23]

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

[commit-bot: I haz the power, 2017-05-31 06:52:37]

Dry run: CQ is trying da patch.

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

[tbansal1, 2017-05-31 06:53:55]

Patchset #2 (id:60001) has been deleted

[commit-bot: I haz the power, 2017-05-31 07:52:18]

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

[commit-bot: I haz the power, 2017-05-31 07:52:19]

Dry run: This issue passed the CQ dry run.

[tbansal1, 2017-05-31 16:03:39]

tbansal@chromium.org changed reviewers:
+ rajendrant@chromium.org, ryansturm@chromium.org

[tbansal1, 2017-05-31 16:03:40]

ryansturm: ptal.

TBRing: rajendrant@

[tbansal1, 2017-05-31 16:03:57]

Description was changed from

==========
PlzNavigate support in data use ascriber

With PlzNavigate, RenderFrameHost will not be populated in mainframe
requests. So global request ID alone should be used for mapping those
requests to main renderframe, when navigation commits later. This CL is 
based off of Chromium CL 2874353002.

BUG=664233
==========

to

==========
PlzNavigate support in data use ascriber

With PlzNavigate, RenderFrameHost will not be populated in mainframe
requests. So global request ID alone should be used for mapping those
requests to main renderframe, when navigation commits later. This CL is 
based off of Chromium CL 2874353002.

BUG=664233
TBR=rajendrant@chromium.org
==========

[RyanSturm, 2017-05-31 16:26:01]

lgtm % comments. Right now the comments aren't super important.

https://codereview.chromium.org/2913063002/diff/120001/chrome/browser/data_us...
File chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc (right):

https://codereview.chromium.org/2913063002/diff/120001/chrome/browser/data_us...
chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc:98: // Create a
new DataUseRecorder for all other requests.
nit: s/other/non-content initiated/

https://codereview.chromium.org/2913063002/diff/120001/chrome/browser/data_us...
chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc:415: void
ChromeDataUseAscriber::DeleteFromMainRenderFrameDataUseMap(
Can't you just call map.delete[entry.main_frame_id()] assuming you can get this
as a ChromeDataUseRecorder or can move the method to DataUseRecorder?

[tbansal1, 2017-05-31 16:32:51]

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

[tbansal1, 2017-05-31 16:32:54]

https://codereview.chromium.org/2913063002/diff/120001/chrome/browser/data_us...
File chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc (right):

https://codereview.chromium.org/2913063002/diff/120001/chrome/browser/data_us...
chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc:98: // Create a
new DataUseRecorder for all other requests.
On 2017/05/31 16:26:01, RyanSturm wrote:
> nit: s/other/non-content initiated/

Done.

https://codereview.chromium.org/2913063002/diff/120001/chrome/browser/data_us...
chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc:415: void
ChromeDataUseAscriber::DeleteFromMainRenderFrameDataUseMap(
On 2017/05/31 16:26:01, RyanSturm wrote:
> Can't you just call map.delete[entry.main_frame_id()] assuming you can get
this
> as a ChromeDataUseRecorder or can move the method to DataUseRecorder?

No, this is called when the iterator is from |pending_navigation_data_use_map_|
which IIUC does not know about |RenderFrameHostID| which is used as key in the
map here.

[commit-bot: I haz the power, 2017-05-31 16:33:22]

Dry run: CQ is trying da patch.

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

[tbansal1, 2017-05-31 16:33:50]

https://codereview.chromium.org/2913063002/diff/120001/chrome/browser/data_us...
File chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc (right):

https://codereview.chromium.org/2913063002/diff/120001/chrome/browser/data_us...
chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc:415: void
ChromeDataUseAscriber::DeleteFromMainRenderFrameDataUseMap(
On 2017/05/31 16:32:54, tbansal1 wrote:
> On 2017/05/31 16:26:01, RyanSturm wrote:
> > Can't you just call map.delete[entry.main_frame_id()] assuming you can get
> this
> > as a ChromeDataUseRecorder or can move the method to DataUseRecorder?
> 
> No, this is called when the iterator is from
|pending_navigation_data_use_map_|
> which IIUC does not know about |RenderFrameHostID| which is used as key in the
> map here.

Ooh, I see. let me try that.

[tbansal1, 2017-05-31 16:40:15]

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

[commit-bot: I haz the power, 2017-05-31 16:40:51]

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-31 18:25:53]

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

[commit-bot: I haz the power, 2017-05-31 18:25:54]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tbansal1, 2017-05-31 18:32:34]

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

[commit-bot: I haz the power, 2017-05-31 18:33: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, 2017-05-31 20:10:53]

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

[commit-bot: I haz the power, 2017-05-31 20:10:54]

Dry run: This issue passed the CQ dry run.

[tbansal1, 2017-05-31 20:11:36]

The CQ bit was checked by tbansal@chromium.org

[tbansal1, 2017-05-31 20:11:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
ryansturm@chromium.org
Link to the patchset: https://codereview.chromium.org/2913063002/#ps160001
(title: "ryansturm comments")

[commit-bot: I haz the power, 2017-05-31 20:12:58]

CQ is trying da patch.

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

[tbansal1, 2017-05-31 20:35:49]

tbansal@chromium.org changed reviewers:
+ megjablon@chromium.org

[commit-bot: I haz the power, 2017-05-31 20:40:33]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1496261496879910,
"parent_rev": "2f65325fc076e6e17958c2e4880c1d0d745db5f4", "commit_rev":
"975ba1a456087d8781f61754b747c66dab30c97f"}

[commit-bot: I haz the power, 2017-05-31 20:40:48]

Description was changed from

==========
PlzNavigate support in data use ascriber

With PlzNavigate, RenderFrameHost will not be populated in mainframe
requests. So global request ID alone should be used for mapping those
requests to main renderframe, when navigation commits later. This CL is 
based off of Chromium CL 2874353002.

BUG=664233
TBR=rajendrant@chromium.org
==========

to

==========
PlzNavigate support in data use ascriber

With PlzNavigate, RenderFrameHost will not be populated in mainframe
requests. So global request ID alone should be used for mapping those
requests to main renderframe, when navigation commits later. This CL is 
based off of Chromium CL 2874353002.

BUG=664233
TBR=rajendrant@chromium.org

Review-Url: https://codereview.chromium.org/2913063002
Cr-Commit-Position: refs/heads/master@{#476006}
Committed:
https://chromium.googlesource.com/chromium/src/+/975ba1a456087d8781f61754b747...
==========

[commit-bot: I haz the power, 2017-05-31 20:40:50]

Committed patchset #4 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/975ba1a456087d8781f61754b747...