Remove workaround for GetLastCommittedOrigin with non-current RFHs.

Remove workaround for GetLastCommittedOrigin with non-current RFHs.

Checking whether the RFH is associated with the frame tree shouldn't
be needed anymore after r437660, which fixed GetLastCommittedOrigin()
to work properly with pending delete RenderFrameHosts.

BUG=663740
Committed: https://crrev.com/9fdb6a4e9076302f38b7d72a8a4fb827dca909c0
Cr-Commit-Position: refs/heads/master@{#439539}

[alexmos, 2016-12-16 17:31:54]

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

[commit-bot: I haz the power, 2016-12-16 17:33:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[alexmos, 2016-12-16 17:38:46]

Description was changed from

==========
Remove workaround for GetLastCommittedOrigin with pending delete RFHs.

Checking whether the RFH is associated with the frame tree shouldn't
be needed anymore after r437660, which fixed GetLastCommittedOrigin()
to work properly with pending delete RenderFrameHosts.

BUG=663740
==========

to

==========
Remove workaround for GetLastCommittedOrigin with non-current RFHs.

Checking whether the RFH is associated with the frame tree shouldn't
be needed anymore after r437660, which fixed GetLastCommittedOrigin()
to work properly with pending delete RenderFrameHosts.

BUG=663740
==========

[alexmos, 2016-12-16 17:39:36]

alexmos@chromium.org changed reviewers:
+ mfoltz@chromium.org

[alexmos, 2016-12-16 17:39:36]

mfoltz@: can you please take a look?   This essentially reverts your
https://codereview.chromium.org/2545523008, which shouldn't be necessary after
my https://codereview.chromium.org/2546533007 fixed GetLastCommittedOrigin to
work with non-current RFHs.

[commit-bot: I haz the power, 2016-12-16 18:45:45]

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

[commit-bot: I haz the power, 2016-12-16 18:45:45]

Dry run: This issue passed the CQ dry run.

[mark a. foltz, 2016-12-16 18:48:00]

mfoltz@chromium.org changed reviewers:
+ imcheng@chromium.org

[mark a. foltz, 2016-12-16 18:48:01]

lgtm

+imcheng as an FYI

To implement the Presentation API, we currently tie a browser-side object to the
render frame host, via the (process_id, routing_id) tuple.  However it sounds
like there may not always be a 1:1 relationship between renderer frames and
renderer fame hosts in all cases.

- Should we track these objects using FrameNodeId instead?

- Is there a better design pattern for maintaining per-frame, browser-side
state?

[alexmos, 2016-12-19 19:14:31]

On 2016/12/16 18:48:01, mark a. foltz wrote:
> lgtm

Thanks!

> +imcheng as an FYI
> 
> To implement the Presentation API, we currently tie a browser-side object to
the
> render frame host, via the (process_id, routing_id) tuple.  However it sounds
> like there may not always be a 1:1 relationship between renderer frames and
> renderer fame hosts in all cases.
> 
> - Should we track these objects using FrameNodeId instead?
> 
> - Is there a better design pattern for maintaining per-frame, browser-side
> state?

So it sounds like you need to maintain an object where the lifetime matches a
frame, so that it stays the same even as the same frame navigates to different
documents? 

Frames (or FrameTreeNodes in the browser process) and RenderFrameHosts are
indeed not 1:1. A cross-process navigation can create a new "pending" RFH (in a
new process) for the same frame, which will become the "current" RenderFrameHost
when the navigation commits.  At commit time, the old RFH switches to a "pending
delete" state until it finishes executing its unload handler and then gets
destroyed.  At any given time, the same frame could have one current RFH, one
pending RFH, and/or multiple pending deletion RFHs.

The best way to deal with this is probably to use
WebContentsObserver::RenderFrameHostChanged to track changes to the current RFH
for a particular frame.  We don't encourage using FrameTreeNodeId for this --
that was added as a way to support some limited cases like webNavigation API,
but we'd like to limit its further use, as it's unclear exactly how we'll want
to expose it down the road.

[alexmos, 2016-12-19 19:14:39]

The CQ bit was checked by alexmos@chromium.org

[commit-bot: I haz the power, 2016-12-19 19:15:18]

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-12-19 20:28:01]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1482174879422650, "parent_rev":
"1ad5d92daadc65550daf03f675b8c99b15619ff5", "commit_rev":
"5d6d409b7ee7acec586837242079457b14f3a8e9"}

[commit-bot: I haz the power, 2016-12-19 20:28:32]

Description was changed from

==========
Remove workaround for GetLastCommittedOrigin with non-current RFHs.

Checking whether the RFH is associated with the frame tree shouldn't
be needed anymore after r437660, which fixed GetLastCommittedOrigin()
to work properly with pending delete RenderFrameHosts.

BUG=663740
==========

to

==========
Remove workaround for GetLastCommittedOrigin with non-current RFHs.

Checking whether the RFH is associated with the frame tree shouldn't
be needed anymore after r437660, which fixed GetLastCommittedOrigin()
to work properly with pending delete RenderFrameHosts.

BUG=663740

Review-Url: https://codereview.chromium.org/2583943002
==========

[commit-bot: I haz the power, 2016-12-19 20:28:34]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-12-19 20:30:18]

Description was changed from

==========
Remove workaround for GetLastCommittedOrigin with non-current RFHs.

Checking whether the RFH is associated with the frame tree shouldn't
be needed anymore after r437660, which fixed GetLastCommittedOrigin()
to work properly with pending delete RenderFrameHosts.

BUG=663740

Review-Url: https://codereview.chromium.org/2583943002
==========

to

==========
Remove workaround for GetLastCommittedOrigin with non-current RFHs.

Checking whether the RFH is associated with the frame tree shouldn't
be needed anymore after r437660, which fixed GetLastCommittedOrigin()
to work properly with pending delete RenderFrameHosts.

BUG=663740

Committed: https://crrev.com/9fdb6a4e9076302f38b7d72a8a4fb827dca909c0
Cr-Commit-Position: refs/heads/master@{#439539}
==========

[commit-bot: I haz the power, 2016-12-19 20:30:19]

Patchset 1 (id:??) landed as
https://crrev.com/9fdb6a4e9076302f38b7d72a8a4fb827dca909c0
Cr-Commit-Position: refs/heads/master@{#439539}