New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 9 users

Issue metadata

Status: Fixed
Owner:
Out until 4th March
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: ----

Blocked on:
issue 418277
issue 425248
issue 473258
issue 486889
issue 486896
issue 492830
issue 543226
issue 543949
issue 544868
issue 545900
issue 548275



Sign in to add a comment
link

Issue 357747: Replace swapped out state of RenderFrameHosts with RenderFrameProxyHost objects

Reported by nasko@chromium.org, Mar 28 2014 Project Member

Issue description

Chromium currently uses the notion of "swapped out" RenderViewHost/RenderFrameHost. This concept exists to support cross-process interactions like postMessage, which need to be able to reference Window objects in its own process, while the real object lives in a separate process.

The existing objects change internal state, which leads to lots of complexities and unneeded code. The goal is to replace the swapped out state with actual type - RenderFrameProxy(Host). This bug will track progress of this migration.
 
Showing comments 34 - 133 of 133 Older

Comment 34 by bugdroid1@chromium.org, Aug 7 2014

Project Member
------------------------------------------------------------------
r287954 | nick@chromium.org | 2014-08-07T03:30:12.116445Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_frame_proxy.cc?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_view_impl.cc?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/frame_host/render_frame_proxy_host.cc?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_frame_proxy.h?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/frame_host/render_frame_proxy_host.h?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/extensions/renderer/script_context.cc?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/frame_messages.h?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_frame_impl.cc?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/frame_host/render_frame_host_impl.cc?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_frame_impl.h?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/frame_host/frame_tree.cc?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/frame_host/render_frame_host_impl.h?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/web_contents/opened_by_dom_browsertest.cc?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/frame_host/frame_tree.h?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/frame_host/frame_tree_browsertest.cc?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/web_contents/web_contents_impl.cc?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/frame_host/render_frame_host_manager.cc?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/web_contents/web_contents_impl.h?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/frame_host/render_frame_host_manager.h?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_thread_impl.cc?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/frame_host/navigation_controller_impl.cc?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/site_per_process_browsertest.cc?r1=287954&r2=287953&pathrev=287954
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_thread_impl.h?r1=287954&r2=287953&pathrev=287954

Start using RenderFrameProxyHost objects.

Instantiate RenderFrameProxyHost objects for remote frames when using
--site-per-process.

BUG= 357747 ,  399709 ,  399775 ,  400594 
TBR=kenrb, creis, kalman

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287557

Review URL: https://codereview.chromium.org/444503002
-----------------------------------------------------------------

Comment 35 by bugdroid1@chromium.org, Aug 22 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/22605793bb04e5a500ac1625fbf64dfab9662a39

commit 22605793bb04e5a500ac1625fbf64dfab9662a39
Author: nasko@chromium.org <nasko@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri Aug 22 17:13:31 2014

Re-enable SitePerProcessBrowserTest.CrossSiteIframe

This CL fixes a few issues in the cross-process subframe navigation and re-enables the CrossSiteIframe test on most platforms. There are still some issues on ChromeOS and Android, which will be investigated and fixed in follow-up CL.

BUG= 399775 ,  357747 

Review URL: https://codereview.chromium.org/479403004

Cr-Commit-Position: refs/heads/master@{#291437}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@291437 0039d316-1c4b-4281-b951-d872f2087c98

Comment 36 by bugdroid1@chromium.org, Aug 22 2014

Project Member
------------------------------------------------------------------
r291437 | nasko@chromium.org | 2014-08-22T17:13:31.634021Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_widget_host_impl.cc?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/web_contents/web_contents_impl_unittest.cc?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_widget_host_impl.h?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/frame_host/render_frame_host_manager.cc?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/view_messages.h?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/site_per_process_browsertest.cc?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_frame_proxy.cc?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_view_impl.cc?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_view_impl.h?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/frame_messages.h?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/applescript/tab_applescript.mm?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_frame_impl.cc?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/frame_host/render_frame_host_impl.cc?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/public/browser/render_widget_host.h?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_frame_impl.h?r1=291437&r2=291436&pathrev=291437
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/frame_host/render_frame_host_impl.h?r1=291437&r2=291436&pathrev=291437

Re-enable SitePerProcessBrowserTest.CrossSiteIframe

This CL fixes a few issues in the cross-process subframe navigation and re-enables the CrossSiteIframe test on most platforms. There are still some issues on ChromeOS and Android, which will be investigated and fixed in follow-up CL.

BUG= 399775 ,  357747 

Review URL: https://codereview.chromium.org/479403004
-----------------------------------------------------------------

Comment 37 by bugdroid1@chromium.org, Aug 28 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b

commit 471be39ef0b50e63cb4861f19016ae9fd3f8273b
Author: benwells <benwells@chromium.org>
Date: Thu Aug 28 07:03:27 2014

Revert of Re-enable SitePerProcessBrowserTest.CrossSiteIframe (patchset #10 of https://codereview.chromium.org/479403004/)

Reason for revert:
This is causing unaddressable memory errors on the DrMemory full bot. Build where the error was introduced:

http://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Content%20Browser%20%28DrMemory%20full%29%20%284%29/builds/945

(error hash: 25DFC5E3C30FC6B7)

Sample log output:

UNADDRESSABLE ACCESS of freed memory: reading 0x034348e0-0x034348e4 4 byte(s)
# 0 blink_web.dll!blink::toCoreFrame                                      [third_party\webkit\source\web\webframe.cpp:24]
# 1 blink_web.dll!blink::RemoteFrameClient::firstChild                    [third_party\webkit\source\web\remoteframeclient.cpp:49]
# 2 blink_web.dll!blink::WebRemoteFrameImpl::~WebRemoteFrameImpl          [third_party\webkit\source\web\webremoteframeimpl.cpp:106]
# 3 blink_web.dll!blink::WebRemoteFrameImpl::close                        [third_party\webkit\source\web\webremoteframeimpl.cpp:131]
# 4 content.dll!content::RenderFrameProxy::~RenderFrameProxy              [content\renderer\render_frame_proxy.cc:120]
# 5 content.dll!content::RenderFrameProxy::`vector deleting destructor'
# 6 content.dll!content::RenderFrameProxy::OnDeleteProxy                  [content\renderer\render_frame_proxy.cc:180]
# 7 content.dll!content::MessageRouter::RouteMessage                      [content\common\message_router.cc:54]
# 8 content.dll!content::MessageRouter::OnMessageReceived                 [content\common\message_router.cc:46]
# 9 content.dll!content::ChildThread::OnMessageReceived                   [content\child\child_thread.cc:494]
#10 ipc.dll!IPC::ChannelProxy::Context::OnDispatchMessage                 [ipc\ipc_channel_proxy.cc:273]
#11 ipc.dll!base::internal::Invoker<>::Run                                [base\bind_internal.h:1253]
#12 base.dll!base::debug::TaskAnnotator::RunTask                          [base\debug\task_annotator.cc:62]
#13 base.dll!base::MessageLoop::RunTask                                   [base\message_loop\message_loop.cc:436]
#14 base.dll!base::MessageLoop::DeferOrRunPendingTask                     [base\message_loop\message_loop.cc:445]
#15 base.dll!base::MessageLoop::DoWork                                    [base\message_loop\message_loop.cc:552]
#16 base.dll!base::MessagePumpDefault::Run                                [base\message_loop\message_pump_default.cc:32]
#17 base.dll!base::MessageLoop::RunHandler                                [base\message_loop\message_loop.cc:408]
#18 content.dll!content::RendererMain                                     [content\renderer\renderer_main.cc:227]
#19 content.dll!content::RunNamedProcessTypeMain                          [content\app\content_main_runner.cc:415]
#20 content.dll!content::ContentMainRunnerImpl::Run                       [content\app\content_main_runner.cc:764]
#21 content.dll!content::ContentMain                                      [content\app\content_main.cc:19]
#22 content::LaunchTests                                                  [content\public\test\test_launcher.cc:475]
#23 main                                                                  [content\test\content_test_launcher.cc:123]

BTW IMHO the CL summary for this change could be better, this change did way more than re-enalbe a test. Ideally the summary would capture the entirety of the change.

Original issue's description:
> Re-enable SitePerProcessBrowserTest.CrossSiteIframe
>
> This CL fixes a few issues in the cross-process subframe navigation and re-enables the CrossSiteIframe test on most platforms. There are still some issues on ChromeOS and Android, which will be investigated and fixed in follow-up CL.
>
> BUG= 399775 ,  357747 
>
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=291437

TBR=creis@chromium.org,avi@chromium.org,nasko@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG= 399775 ,  357747 

Review URL: https://codereview.chromium.org/515073002

Cr-Commit-Position: refs/heads/master@{#292336}

[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/chrome/browser/ui/cocoa/applescript/tab_applescript.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/browser/frame_host/render_frame_host_impl.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/browser/site_per_process_browsertest.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/common/frame_messages.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/common/view_messages.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/public/browser/render_widget_host.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/renderer/render_frame_impl.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/renderer/render_frame_impl.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/renderer/render_frame_proxy.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/renderer/render_view_impl.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/471be39ef0b50e63cb4861f19016ae9fd3f8273b/content/renderer/render_view_impl.h

Comment 39 by bugdroid1@chromium.org, Sep 12 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/faab4288197b20b73610aad3e5579751b0c3a97b

commit faab4288197b20b73610aad3e5579751b0c3a97b
Author: nasko <nasko@chromium.org>
Date: Fri Sep 12 01:59:05 2014

Do not create proxy hosts in the subtree of navigating frame.

When a frame is navigating cross-process, its existing document will be completely destroyed and all child frames will be gone. This means that we don't need to create proxy objects for the new SiteInstance in the subtree of the navigating frame.

BUG= 357747 

Review URL: https://codereview.chromium.org/536143002

Cr-Commit-Position: refs/heads/master@{#294520}

[modify] https://chromium.googlesource.com/chromium/src.git/+/faab4288197b20b73610aad3e5579751b0c3a97b/content/browser/accessibility/site_per_process_accessibility_browsertest.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/faab4288197b20b73610aad3e5579751b0c3a97b/content/browser/frame_host/frame_tree.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/faab4288197b20b73610aad3e5579751b0c3a97b/content/browser/frame_host/frame_tree.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/faab4288197b20b73610aad3e5579751b0c3a97b/content/browser/frame_host/frame_tree_browsertest.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/faab4288197b20b73610aad3e5579751b0c3a97b/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/faab4288197b20b73610aad3e5579751b0c3a97b/content/browser/site_per_process_browsertest.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/faab4288197b20b73610aad3e5579751b0c3a97b/content/test/data/site_per_process_main.html

Comment 40 by bugdroid1@chromium.org, Sep 12 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c328fa4adbb3dc43b921d643ac0cf73fea5c42ef

commit c328fa4adbb3dc43b921d643ac0cf73fea5c42ef
Author: engedy <engedy@chromium.org>
Date: Fri Sep 12 08:56:08 2014

Revert of Do not create proxy hosts in the subtree of navigating frame. (patchset #11 id:200001 of https://codereview.chromium.org/536143002/)

Reason for revert:
SitePerProcessBrowserTest.ProxyCreationSkipsSubtree repeatedly fails on Mac with ASAN reporting use-after-free, see:

http://build.chromium.org/p/chromium.memory/builders/Mac%20ASan%2064%20Tests%20%281%29/builds/2174
http://build.chromium.org/p/chromium.fyi/builders/Chromium%20Mac%2010.8%20Buildrunner/builds/19009

@Nasko, as there are some simultaneous breakages right now, so I could not investigate this further and opted to revert the whole thing to get the tree green ASAP.

Snippets from test log:

ASSERTION FAILED: !localFrame || (localFrame->isLocalFrame())
../../third_party/WebKit/Source/core/frame/LocalFrame.h(253) : blink::LocalFrame *blink::toLocalFrame(blink::Frame *)
../../content/browser/site_per_process_browsertest.cc:690: Failure
Value of: child->child_count()
  Actual: 2
Expected: 0U
Which is: 0

Original issue's description:
> Do not create proxy hosts in the subtree of navigating frame.
>
> When a frame is navigating cross-process, its existing document will be completely destroyed and all child frames will be gone. This means that we don't need to create proxy objects for the new SiteInstance in the subtree of the navigating frame.
>
> BUG= 357747 
>
> Committed: https://crrev.com/faab4288197b20b73610aad3e5579751b0c3a97b
> Cr-Commit-Position: refs/heads/master@{#294520}

TBR=ajwong@chromium.org,kenrb@chromium.org,jam@chromium.org,creis@chromium.org,dmazzoni@chromium.org,nasko@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG= 357747 

Review URL: https://codereview.chromium.org/565103002

Cr-Commit-Position: refs/heads/master@{#294552}

[modify] https://chromium.googlesource.com/chromium/src.git/+/c328fa4adbb3dc43b921d643ac0cf73fea5c42ef/content/browser/accessibility/site_per_process_accessibility_browsertest.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/c328fa4adbb3dc43b921d643ac0cf73fea5c42ef/content/browser/frame_host/frame_tree.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/c328fa4adbb3dc43b921d643ac0cf73fea5c42ef/content/browser/frame_host/frame_tree.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/c328fa4adbb3dc43b921d643ac0cf73fea5c42ef/content/browser/frame_host/frame_tree_browsertest.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/c328fa4adbb3dc43b921d643ac0cf73fea5c42ef/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/c328fa4adbb3dc43b921d643ac0cf73fea5c42ef/content/browser/site_per_process_browsertest.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/c328fa4adbb3dc43b921d643ac0cf73fea5c42ef/content/test/data/site_per_process_main.html

Comment 41 by nasko@chromium.org, Sep 22 2014

Currently RenderFrameProxy objects can be deleted in response to IPCs sent from the browser process. The handler for the FrameMsg_DeleteProxy IPC deletes the proxy object, which calls close() on the underlying WebRemoteFrame. The effect of this is that both the RenderFrameProxy and WebRemoteFrame objects are deleted, but the frame is not removed from the frame tree in the renderer. This leads to use-after-free bug and should be fixed.

The tentative plan is to mirror how RenderFrame/LocalFrame is cleaned up, where during detach Blink calls out to content/ to clean up the WebFrameClient. Once that is in place, RenderFrameProxy will initiate detach, which in turn will cause it to delete itself.

Comment 42 by bugdroid1@chromium.org, Sep 24 2014

Project Member
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=182625

------------------------------------------------------------------
r182625 | dcheng@chromium.org | 2014-09-24T18:24:22.394803Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/public/web/WebRemoteFrameClient.h?r1=182625&r2=182624&pathrev=182625
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebFrame.cpp?r1=182625&r2=182624&pathrev=182625
   M http://src.chromium.org/viewvc/blink/trunk/public/web/WebFrame.h?r1=182625&r2=182624&pathrev=182625

Expose Frame::detach() to the Blink API layer.

This is needed when the embedder needs to trigger frame detach for frame
trees that have been mirrored into other processes. This patch also adds
a frameDetached() callback for remote frames so the embedder knows when
to close() the corresponding frame.

BUG= 357747 

Review URL: https://codereview.chromium.org/593893002
-----------------------------------------------------------------

Comment 43 by creis@chromium.org, Sep 26 2014

Blockedon: chromium:418277

Comment 44 by bugdroid1@chromium.org, Oct 17 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e6edde315c6cb57cae5f67056191dceab3f7403b

commit e6edde315c6cb57cae5f67056191dceab3f7403b
Author: nasko <nasko@chromium.org>
Date: Fri Oct 17 15:36:48 2014

Do not create proxy hosts in the subtree of navigating frame.

This is an another attempt at landing https://codereview.chromium.org/536143002.

When a frame is navigating cross-process, its existing document will be completely destroyed and all child frames will be gone. This means that we don't need to create proxy objects for the new SiteInstance in the subtree of the navigating frame.

BUG= 357747 , 417030

Review URL: https://codereview.chromium.org/600483003

Cr-Commit-Position: refs/heads/master@{#300112}

[modify] https://chromium.googlesource.com/chromium/src.git/+/e6edde315c6cb57cae5f67056191dceab3f7403b/content/browser/accessibility/site_per_process_accessibility_browsertest.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/e6edde315c6cb57cae5f67056191dceab3f7403b/content/browser/frame_host/frame_tree.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/e6edde315c6cb57cae5f67056191dceab3f7403b/content/browser/frame_host/frame_tree.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/e6edde315c6cb57cae5f67056191dceab3f7403b/content/browser/frame_host/frame_tree_browsertest.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/e6edde315c6cb57cae5f67056191dceab3f7403b/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/e6edde315c6cb57cae5f67056191dceab3f7403b/content/browser/site_per_process_browsertest.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/e6edde315c6cb57cae5f67056191dceab3f7403b/content/test/data/site_per_process_main.html

Comment 45 by dgro...@chromium.org, Oct 20 2014

Blockedon: chromium:425248

Comment 46 by bugdroid1@chromium.org, Nov 25 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bd91133930a1f13f3b2453d4a4f04cfdcab09210

commit bd91133930a1f13f3b2453d4a4f04cfdcab09210
Author: nasko <nasko@chromium.org>
Date: Tue Nov 25 01:13:36 2014

Refactor RenderView creation to use ViewMsg_New_Params.

Using a RenderViewImplParams struct to create RenderViewImpl objects was introduced in crrev.com/167341. It doesn't look necessary anymore, so I'm removing it as it adds an unnecessary step of converting one struct to another identicial one.

BUG= 357747 
TEST=Layout tests and content_browsertests should still work.

TBR=jochen@chromium.org

Review URL: https://codereview.chromium.org/751043002

Cr-Commit-Position: refs/heads/master@{#305545}

[modify] http://crrev.com/bd91133930a1f13f3b2453d4a4f04cfdcab09210/content/content_renderer.gypi
[modify] http://crrev.com/bd91133930a1f13f3b2453d4a4f04cfdcab09210/content/public/test/render_view_test.cc
[modify] http://crrev.com/bd91133930a1f13f3b2453d4a4f04cfdcab09210/content/renderer/render_thread_impl.cc
[modify] http://crrev.com/bd91133930a1f13f3b2453d4a4f04cfdcab09210/content/renderer/render_view_impl.cc
[modify] http://crrev.com/bd91133930a1f13f3b2453d4a4f04cfdcab09210/content/renderer/render_view_impl.h
[delete] http://crrev.com/1c84cb86b5d4f82f50c75c1427b2c1994a485b36/content/renderer/render_view_impl_params.cc
[delete] http://crrev.com/1c84cb86b5d4f82f50c75c1427b2c1994a485b36/content/renderer/render_view_impl_params.h
[modify] http://crrev.com/bd91133930a1f13f3b2453d4a4f04cfdcab09210/content/test/layouttest_support.cc

Comment 47 by bugdroid1@chromium.org, Feb 14 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/912887b4a462f216fca73228de49b4c6cc980da4

commit 912887b4a462f216fca73228de49b4c6cc980da4
Author: lfg <lfg@chromium.org>
Date: Sat Feb 14 00:04:00 2015

Sets render_frame_proxy_ to null in the RenderFrameImpl when destroying the RenderFrameProxy.

This fixes with a use-after-free in the RenderFrameProxy reported by the asan bots, but uncovers another one.

TEST=NavigateRemoteFrame
BUG= 357747 

Review URL: https://codereview.chromium.org/929463004

Cr-Commit-Position: refs/heads/master@{#316337}

[modify] http://crrev.com/912887b4a462f216fca73228de49b4c6cc980da4/content/renderer/render_frame_proxy.cc

Comment 48 by bugdroid1@chromium.org, May 6 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8b9d9bd3f88f4ea830a15120d7833557193843de

commit 8b9d9bd3f88f4ea830a15120d7833557193843de
Author: nasko <nasko@chromium.org>
Date: Wed May 06 19:23:19 2015

Add a member boolean to indicate whether RenderFrameImpl is subframe.

Whether a RenderFrameImpl is a subframe or the main frame is known at
initialization time and doesn't change for the lifetime of the object.
This CL adds a boolean that keeps that state and removes the few local
variables used for checking whether it is a subframe. The reason is
that as we invert the ownership of RenderViewImpl and RenderFrameImpl,
the main frame will have to pass ownership of the RenderViewImpl to
the RenderFrameProxy during swapping local to remote. Since the frame
will be destroyed, it needs to instruct RenderViewImpl to clear its
main_render_frame_ pointer, because it is going to be invalid.

BUG= 357747 

Review URL: https://codereview.chromium.org/1125243002

Cr-Commit-Position: refs/heads/master@{#328579}

[modify] http://crrev.com/8b9d9bd3f88f4ea830a15120d7833557193843de/content/renderer/render_frame_impl.cc
[modify] http://crrev.com/8b9d9bd3f88f4ea830a15120d7833557193843de/content/renderer/render_frame_impl.h
[modify] http://crrev.com/8b9d9bd3f88f4ea830a15120d7833557193843de/content/renderer/render_frame_impl_browsertest.cc

Comment 49 by bugdroid1@chromium.org, May 8 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ef4c112d2d588c58e44b7566b93bd6b26ddaaedd

commit ef4c112d2d588c58e44b7566b93bd6b26ddaaedd
Author: nasko <nasko@chromium.org>
Date: Fri May 08 01:17:43 2015

Delete entry in WebFrame->RenderFrameProxy map at WebFrame::close time.

The entry in the WebFrame->RenderFrameProxy map is removed in the
RenderFrameProxy destructor, but the destructor runs after the frame
has been destroyed. Remove the entry at the time of WebFrame::close().

BUG= 357747 

Review URL: https://codereview.chromium.org/1136593003

Cr-Commit-Position: refs/heads/master@{#328897}

[modify] http://crrev.com/ef4c112d2d588c58e44b7566b93bd6b26ddaaedd/content/renderer/render_frame_impl.cc
[modify] http://crrev.com/ef4c112d2d588c58e44b7566b93bd6b26ddaaedd/content/renderer/render_frame_proxy.cc

Comment 50 by bugdroid1@chromium.org, May 8 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/63b8975f954f2e10ee8c1b339c00b2b252f46132

commit 63b8975f954f2e10ee8c1b339c00b2b252f46132
Author: nasko <nasko@chromium.org>
Date: Fri May 08 18:23:06 2015

MainFrameObserver is inline owned, so it shouldn't be freed by OnDestruct

MainFrameObserver is a RenderFrameObserver and as such is destroyed when
RenderFrame goes away. This happens as part of
RenderFrameObserver::OnDestruct. MainFrameObserver though is inline owned
by WebUIMojo and can be deleted before WebUIMojo is deleted. It results in
use-after-free when WebUIMojo is destructed and tries to free the already
freed MainFrameObserver.

This CL overrides OnDestruct, which allows the MainFrameObserver to stay
alive and be cleaned up by WebUIMojo.

BUG= 357747 

Review URL: https://codereview.chromium.org/1137533002

Cr-Commit-Position: refs/heads/master@{#328990}

[modify] http://crrev.com/63b8975f954f2e10ee8c1b339c00b2b252f46132/content/renderer/web_ui_mojo.cc
[modify] http://crrev.com/63b8975f954f2e10ee8c1b339c00b2b252f46132/content/renderer/web_ui_mojo.h

Comment 51 by bugdroid1@chromium.org, May 8 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c27ac70fabb2f93aeb486b783c170bbaee7f7304

commit c27ac70fabb2f93aeb486b783c170bbaee7f7304
Author: nasko <nasko@chromium.org>
Date: Fri May 08 18:28:04 2015

Remove unneeded AddRef in RenderViewTest::SetUp

The AddRef in RenderViewTest::SetUp is not needed and can lead to leaks
in tests. RenderView takes a reference on itself during initialization
and drops it in RenderWidget::Close (which it inherits & calls directly).

TBR=jam@chromium.org
BUG= 357747 

Review URL: https://codereview.chromium.org/1135643002

Cr-Commit-Position: refs/heads/master@{#328991}

[modify] http://crrev.com/c27ac70fabb2f93aeb486b783c170bbaee7f7304/content/public/test/render_view_test.cc

Comment 52 by dcheng@chromium.org, May 8 2015

Issue 475003 has been merged into this issue.

Comment 53 by nasko@chromium.org, May 11 2015

Blockedon: chromium:473258

Comment 54 by nasko@chromium.org, May 11 2015

Blockedon: chromium:486889

Comment 55 by nasko@chromium.org, May 11 2015

Blockedon: chromium:486896

Comment 56 by bugdroid1@chromium.org, May 12 2015

Project Member
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=195211

------------------------------------------------------------------
r195211 | dcheng@chromium.org | 2015-05-12T00:16:46.456997Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebRemoteFrameImpl.cpp?r1=195211&r2=195210&pathrev=195211
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/Frame.cpp?r1=195211&r2=195210&pathrev=195211

Fix Frame::isMainFrame() to be compatible with provisional frames

In OOPI, there can be provisional local frames. These frames are only
partially linked into the frame tree. A provisional child frame has a
parent set, but it does not appear in its parent's list of children.
Similarly, a provisional main frame has a null parent set, but is never
set as the main frame of a Page.

Frame::isMainFrame() originally checked if Page::mainFrame() == this to
determine if the frame is a main frame. However, code in navigation
asserts that if Frame::isMainFrame() is false, the frame has a parent,
which fails for provisional local main frames. They have no parent
frame, but they are not yet set as the main frame of the Page: they only
become the Page's main frame if the load commits.

Since the parent is properly set for provisional local frames, change
Frame::isMainFrame() to use that to determine main frame status.

BUG= 357747 

Review URL: https://codereview.chromium.org/1128403004
-----------------------------------------------------------------

Comment 57 by bugdroid1@chromium.org, May 12 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/77de231e243d5adb0bd82c630fd6f3382ab3f42e

commit 77de231e243d5adb0bd82c630fd6f3382ab3f42e
Author: nasko <nasko@chromium.org>
Date: Tue May 12 03:09:16 2015

Convert main_render_frame_ to raw pointer in RenderViewImpl.

The main RenderFrame object for a page is currently owned directly
through a scoped_ptr by RenderViewImpl. Since we want the main
RenderFrame to be deleted when we do a cross-process navigation,
it needs to be cleaned by as part of the regular callbacks from
Blink - WebFrameClient::frameDetached. This requires the frame to not
be owned by RenderViewImpl, so this CL changes it from scoped_ptr
to raw pointer.

The only change in behavior after this CL is that the RenderFrame
will be destroyed prior to RenderView being destroyed as part of
WebView::close(). The destruction of both objects still stays in
the same task on the event loop, though.

BUG= 357747 

Review URL: https://codereview.chromium.org/1130233002

Cr-Commit-Position: refs/heads/master@{#329337}

[modify] http://crrev.com/77de231e243d5adb0bd82c630fd6f3382ab3f42e/content/renderer/render_frame_impl.cc
[modify] http://crrev.com/77de231e243d5adb0bd82c630fd6f3382ab3f42e/content/renderer/render_view_browsertest.cc
[modify] http://crrev.com/77de231e243d5adb0bd82c630fd6f3382ab3f42e/content/renderer/render_view_impl.cc
[modify] http://crrev.com/77de231e243d5adb0bd82c630fd6f3382ab3f42e/content/renderer/render_view_impl.h

Comment 58 by bugdroid1@chromium.org, May 21 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/25c15f366899cea3ec6d72904cb97d61d78e79f3

commit 25c15f366899cea3ec6d72904cb97d61d78e79f3
Author: nasko <nasko@chromium.org>
Date: Thu May 21 19:47:15 2015

Ensure WebUI bindings are properly set

When a RenderViewHost is created in swapped out state, it doesn't have
any WebUI bindings. If later on a navigation is performed that uses
the same RenderViewHost, we need to ensure it has WebUI bindings.
Currently the code does that conditionally inside RFHM::CreateRenderFrame
and it should be done unconditionally.

BUG= 357747 
TEST=Existing browser tests pass, including WebUIGetsBindings

Review URL: https://codereview.chromium.org/1145353002

Cr-Commit-Position: refs/heads/master@{#330956}

[modify] http://crrev.com/25c15f366899cea3ec6d72904cb97d61d78e79f3/content/browser/frame_host/render_frame_host_manager.cc

Comment 59 by bugdroid1@chromium.org, May 21 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8542185bbb9fd0c0e7d0240c64403d2e4cea39f2

commit 8542185bbb9fd0c0e7d0240c64403d2e4cea39f2
Author: nasko <nasko@chromium.org>
Date: Thu May 21 22:53:48 2015

Add ref-count on RenderViewHost for each new RenderFrameProxyHost.

Currently, only RenderFrameHosts are ref-counting the associated RenderViewHost.
Since it is possible to have a RenderViewHost with no associated RenderFrameHost
(for example a cross-process window opener/openee relationship), it should
be taken into account that the associated RenderFrameProxyHost is also
a ref-counting the RenderViewHost.
The goal of this CL is to allow for that, even though it isn't currently
the case. It is a small step towards removing swapped out RenderFrameHosts.

BUG= 357747 

Review URL: https://codereview.chromium.org/1150793002

Cr-Commit-Position: refs/heads/master@{#331016}

[modify] http://crrev.com/8542185bbb9fd0c0e7d0240c64403d2e4cea39f2/content/browser/frame_host/frame_tree.cc
[modify] http://crrev.com/8542185bbb9fd0c0e7d0240c64403d2e4cea39f2/content/browser/frame_host/frame_tree.h
[modify] http://crrev.com/8542185bbb9fd0c0e7d0240c64403d2e4cea39f2/content/browser/frame_host/render_frame_host_impl.cc
[modify] http://crrev.com/8542185bbb9fd0c0e7d0240c64403d2e4cea39f2/content/browser/frame_host/render_frame_host_manager.cc
[modify] http://crrev.com/8542185bbb9fd0c0e7d0240c64403d2e4cea39f2/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] http://crrev.com/8542185bbb9fd0c0e7d0240c64403d2e4cea39f2/content/browser/frame_host/render_frame_proxy_host.cc
[modify] http://crrev.com/8542185bbb9fd0c0e7d0240c64403d2e4cea39f2/content/browser/frame_host/render_frame_proxy_host.h
[modify] http://crrev.com/8542185bbb9fd0c0e7d0240c64403d2e4cea39f2/content/browser/site_per_process_browsertest.cc

Comment 60 by bugdroid1@chromium.org, May 22 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/72e13b2f01beb6105c6a74b66aa92bdc6d553a8e

commit 72e13b2f01beb6105c6a74b66aa92bdc6d553a8e
Author: holte <holte@chromium.org>
Date: Fri May 22 01:30:43 2015

Revert of Add ref-count on RenderViewHost for each new RenderFrameProxyHost. (patchset #4 id:60001 of https://codereview.chromium.org/1150793002/)

Reason for revert:
It looks like the new browser test fails on Win8 aura https://build.chromium.org/p/chromium.win/builders/Win8%20Aura/builds/30026/steps/content_browsertests/logs/RenderFrameHostManagerTest.CleanupOnCrossProcessNavigation

Original issue's description:
> Add ref-count on RenderViewHost for each new RenderFrameProxyHost.
>
> Currently, only RenderFrameHosts are ref-counting the associated RenderViewHost.
> Since it is possible to have a RenderViewHost with no associated RenderFrameHost
> (for example a cross-process window opener/openee relationship), it should
> be taken into account that the associated RenderFrameProxyHost is also
> a ref-counting the RenderViewHost.
> The goal of this CL is to allow for that, even though it isn't currently
> the case. It is a small step towards removing swapped out RenderFrameHosts.
>
> BUG= 357747 
>
> Committed: https://crrev.com/8542185bbb9fd0c0e7d0240c64403d2e4cea39f2
> Cr-Commit-Position: refs/heads/master@{#331016}

TBR=creis@chromium.org,nasko@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 357747 

Review URL: https://codereview.chromium.org/1157563002

Cr-Commit-Position: refs/heads/master@{#331041}

[modify] http://crrev.com/72e13b2f01beb6105c6a74b66aa92bdc6d553a8e/content/browser/frame_host/frame_tree.cc
[modify] http://crrev.com/72e13b2f01beb6105c6a74b66aa92bdc6d553a8e/content/browser/frame_host/frame_tree.h
[modify] http://crrev.com/72e13b2f01beb6105c6a74b66aa92bdc6d553a8e/content/browser/frame_host/render_frame_host_impl.cc
[modify] http://crrev.com/72e13b2f01beb6105c6a74b66aa92bdc6d553a8e/content/browser/frame_host/render_frame_host_manager.cc
[modify] http://crrev.com/72e13b2f01beb6105c6a74b66aa92bdc6d553a8e/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] http://crrev.com/72e13b2f01beb6105c6a74b66aa92bdc6d553a8e/content/browser/frame_host/render_frame_proxy_host.cc
[modify] http://crrev.com/72e13b2f01beb6105c6a74b66aa92bdc6d553a8e/content/browser/frame_host/render_frame_proxy_host.h
[modify] http://crrev.com/72e13b2f01beb6105c6a74b66aa92bdc6d553a8e/content/browser/site_per_process_browsertest.cc

Comment 61 by bugdroid1@chromium.org, May 22 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/30663b189dafe37e6dd94f73a1d88d9efe8fe787

commit 30663b189dafe37e6dd94f73a1d88d9efe8fe787
Author: nasko <nasko@chromium.org>
Date: Fri May 22 17:44:25 2015

Add tests to ensure no RFH/RVH objects leak on cross-process navigation

These tests were written as part of https://codereview.chromium.org/1150793002/
which failed on Win8 Aura. Splitting only the test into a separate CL
so it can be committed independently.

BUG= 357747 

Review URL: https://codereview.chromium.org/1145073004

Cr-Commit-Position: refs/heads/master@{#331138}

[modify] http://crrev.com/30663b189dafe37e6dd94f73a1d88d9efe8fe787/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] http://crrev.com/30663b189dafe37e6dd94f73a1d88d9efe8fe787/content/browser/site_per_process_browsertest.cc

Comment 62 by bugdroid1@chromium.org, May 22 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/13ab9e08366bd255c00f2a5b23a9e75933ec1dd7

commit 13ab9e08366bd255c00f2a5b23a9e75933ec1dd7
Author: holte <holte@chromium.org>
Date: Fri May 22 20:06:40 2015

Revert of Add tests to ensure no RFH/RVH objects leak on cross-process navigation (patchset #2 id:20001 of https://codereview.chromium.org/1145073004/)

Reason for revert:
Still causes failures, on both Mac and Win8 Aura

https://build.chromium.org/p/chromium.mac/builders/Mac10.9%20Tests/builds/2165/steps/content_browsertests%20on%20Mac-10.9/logs/RenderFrameHostManagerTest.CleanupOnCrossProcessNavigation

Original issue's description:
> Add tests to ensure no RFH/RVH objects leak on cross-process navigation
>
> These tests were written as part of https://codereview.chromium.org/1150793002/
> which failed on Win8 Aura. Splitting only the test into a separate CL
> so it can be committed independently.
>
> BUG= 357747 
>
> Committed: https://crrev.com/30663b189dafe37e6dd94f73a1d88d9efe8fe787
> Cr-Commit-Position: refs/heads/master@{#331138}

TBR=creis@chromium.org,nasko@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 357747 

Review URL: https://codereview.chromium.org/1149883003

Cr-Commit-Position: refs/heads/master@{#331163}

[modify] http://crrev.com/13ab9e08366bd255c00f2a5b23a9e75933ec1dd7/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] http://crrev.com/13ab9e08366bd255c00f2a5b23a9e75933ec1dd7/content/browser/site_per_process_browsertest.cc

Comment 63 by bugdroid1@chromium.org, May 23 2015

Project Member

Comment 64 by nasko@chromium.org, May 27 2015

Blockedon: chromium:492830

Comment 65 by bugdroid1@chromium.org, May 29 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c88002d1c4dc328fbe519dac9a88ba00396836b4

commit c88002d1c4dc328fbe519dac9a88ba00396836b4
Author: nasko <nasko@chromium.org>
Date: Fri May 29 00:38:23 2015

Bring RFH/RVH unit tests closer to reality of how RF/RV are initialized

BUG= 357747 
TEST=all unit tests continue passing
TBR=sky@chromium.org

Review URL: https://codereview.chromium.org/1151973005

Cr-Commit-Position: refs/heads/master@{#331894}

[modify] http://crrev.com/c88002d1c4dc328fbe519dac9a88ba00396836b4/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
[modify] http://crrev.com/c88002d1c4dc328fbe519dac9a88ba00396836b4/components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc
[modify] http://crrev.com/c88002d1c4dc328fbe519dac9a88ba00396836b4/content/browser/frame_host/frame_tree_unittest.cc
[modify] http://crrev.com/c88002d1c4dc328fbe519dac9a88ba00396836b4/content/browser/frame_host/navigation_controller_impl_unittest.cc
[modify] http://crrev.com/c88002d1c4dc328fbe519dac9a88ba00396836b4/content/browser/frame_host/render_frame_host_impl.cc
[modify] http://crrev.com/c88002d1c4dc328fbe519dac9a88ba00396836b4/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
[modify] http://crrev.com/c88002d1c4dc328fbe519dac9a88ba00396836b4/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] http://crrev.com/c88002d1c4dc328fbe519dac9a88ba00396836b4/content/test/test_render_frame_host.cc
[modify] http://crrev.com/c88002d1c4dc328fbe519dac9a88ba00396836b4/content/test/test_render_frame_host.h
[modify] http://crrev.com/c88002d1c4dc328fbe519dac9a88ba00396836b4/content/test/test_render_view_host.cc
[modify] http://crrev.com/c88002d1c4dc328fbe519dac9a88ba00396836b4/content/test/test_web_contents.cc

Comment 66 by bugdroid1@chromium.org, May 30 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0cbda411462a1adef17e550c2db22c57dedae810

commit 0cbda411462a1adef17e550c2db22c57dedae810
Author: thakis <thakis@chromium.org>
Date: Sat May 30 07:03:32 2015

Revert of Bring RFH/RVH unit tests closer to reality of how RF/RV are initialized (patchset #8 id:130001 of https://codereview.chromium.org/1151973005/)

Reason for revert:
Speculative, might have caused http://crbug.com/493584 (RlzLibTests failing in unit_tests in official builds)

Original issue's description:
> Bring RFH/RVH unit tests closer to reality of how RF/RV are initialized
>
> BUG= 357747 
> TEST=all unit tests continue passing
> TBR=sky@chromium.org
>
> Committed: https://crrev.com/c88002d1c4dc328fbe519dac9a88ba00396836b4
> Cr-Commit-Position: refs/heads/master@{#331894}

TBR=nick@chromium.org,bengr@chromium.org,sky@chromium.org,nasko@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 357747 

Review URL: https://codereview.chromium.org/1146403009

Cr-Commit-Position: refs/heads/master@{#332117}

[modify] http://crrev.com/0cbda411462a1adef17e550c2db22c57dedae810/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
[modify] http://crrev.com/0cbda411462a1adef17e550c2db22c57dedae810/components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc
[modify] http://crrev.com/0cbda411462a1adef17e550c2db22c57dedae810/content/browser/frame_host/frame_tree_unittest.cc
[modify] http://crrev.com/0cbda411462a1adef17e550c2db22c57dedae810/content/browser/frame_host/navigation_controller_impl_unittest.cc
[modify] http://crrev.com/0cbda411462a1adef17e550c2db22c57dedae810/content/browser/frame_host/render_frame_host_impl.cc
[modify] http://crrev.com/0cbda411462a1adef17e550c2db22c57dedae810/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
[modify] http://crrev.com/0cbda411462a1adef17e550c2db22c57dedae810/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] http://crrev.com/0cbda411462a1adef17e550c2db22c57dedae810/content/test/test_render_frame_host.cc
[modify] http://crrev.com/0cbda411462a1adef17e550c2db22c57dedae810/content/test/test_render_frame_host.h
[modify] http://crrev.com/0cbda411462a1adef17e550c2db22c57dedae810/content/test/test_render_view_host.cc
[modify] http://crrev.com/0cbda411462a1adef17e550c2db22c57dedae810/content/test/test_web_contents.cc

Comment 67 by bugdroid1@chromium.org, Jun 2 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c95dcf5164de33fd42d29567adb24655b4c2ab0

commit 8c95dcf5164de33fd42d29567adb24655b4c2ab0
Author: nasko <nasko@chromium.org>
Date: Tue Jun 02 13:44:48 2015

Bring RFH/RVH unit tests closer to reality of how RF/RV are initialized

This is an attempt at relanding https://codereview.chromium.org/1151973005,
which broke official builds due to RlzLibTest unit tests failing.
The initial patchset is identical to the previously committed CL and
subsequent uploads are changes made to fix the failures.

BUG= 357747 
TEST=all unit tests continue passing

Review URL: https://codereview.chromium.org/1156023006

Cr-Commit-Position: refs/heads/master@{#332380}

[modify] http://crrev.com/8c95dcf5164de33fd42d29567adb24655b4c2ab0/chrome/browser/rlz/rlz_unittest.cc
[modify] http://crrev.com/8c95dcf5164de33fd42d29567adb24655b4c2ab0/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
[modify] http://crrev.com/8c95dcf5164de33fd42d29567adb24655b4c2ab0/components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc
[modify] http://crrev.com/8c95dcf5164de33fd42d29567adb24655b4c2ab0/content/browser/frame_host/frame_tree_unittest.cc
[modify] http://crrev.com/8c95dcf5164de33fd42d29567adb24655b4c2ab0/content/browser/frame_host/navigation_controller_impl_unittest.cc
[modify] http://crrev.com/8c95dcf5164de33fd42d29567adb24655b4c2ab0/content/browser/frame_host/render_frame_host_impl.cc
[modify] http://crrev.com/8c95dcf5164de33fd42d29567adb24655b4c2ab0/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
[modify] http://crrev.com/8c95dcf5164de33fd42d29567adb24655b4c2ab0/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] http://crrev.com/8c95dcf5164de33fd42d29567adb24655b4c2ab0/content/public/test/test_renderer_host.h
[modify] http://crrev.com/8c95dcf5164de33fd42d29567adb24655b4c2ab0/content/test/test_render_frame_host.cc
[modify] http://crrev.com/8c95dcf5164de33fd42d29567adb24655b4c2ab0/content/test/test_render_frame_host.h
[modify] http://crrev.com/8c95dcf5164de33fd42d29567adb24655b4c2ab0/content/test/test_render_view_host.cc
[modify] http://crrev.com/8c95dcf5164de33fd42d29567adb24655b4c2ab0/content/test/test_web_contents.cc

Comment 68 by bugdroid1@chromium.org, Jun 2 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/954b02627a8c041cc5d3af2ae57593407bd4570b

commit 954b02627a8c041cc5d3af2ae57593407bd4570b
Author: nasko <nasko@chromium.org>
Date: Tue Jun 02 21:02:31 2015

Improve process crash handling in RenderViewHost & mock RenderProcessHost.

Currently the mock RenderProcessHost always lies about having a connection
to the renderer process. This CL aims to make it more realistic by keeping
track of process initialization and crashing.

Additionally, RenderViewHost can directly monitor the RenderProcessHost
for crashes instead of relying on the RenderFrameHost to notify it.
The CL includes the decoupling of the two.

BUG= 357747 

Review URL: https://codereview.chromium.org/1159183002

Cr-Commit-Position: refs/heads/master@{#332462}

[modify] http://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b/content/browser/frame_host/frame_tree_unittest.cc
[modify] http://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b/content/browser/frame_host/render_frame_host_impl.cc
[modify] http://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b/content/browser/frame_host/render_frame_host_manager.cc
[modify] http://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b/content/browser/frame_host/render_frame_host_manager_unittest.cc
[modify] http://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b/content/browser/renderer_host/render_view_host_impl.cc
[modify] http://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b/content/browser/renderer_host/render_view_host_impl.h
[modify] http://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b/content/browser/renderer_host/render_widget_host_impl.cc
[modify] http://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] http://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm
[modify] http://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] http://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b/content/public/test/mock_render_process_host.cc
[modify] http://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b/content/public/test/mock_render_process_host.h
[modify] http://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b/content/public/test/test_renderer_host.cc
[modify] http://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b/ui/views/controls/webview/webview_interactive_uitest.cc

Comment 69 by bugdroid1@chromium.org, Jun 5 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c0feb652441719f6410ac137ef42b51c7c14a6e

commit 4c0feb652441719f6410ac137ef42b51c7c14a6e
Author: nasko <nasko@chromium.org>
Date: Fri Jun 05 18:37:06 2015

Remove swapped-out usage in --site-per-process.

When navigating cross-process, we use swapped-out RenderFrameHost as placeholders. The goal is to replace this with RenderFrameProxyHost, which is already the case for all subframes, but not the main frame. The code is far along that this can be done now for the main frame as well.

This CL starts the path to removing the swapped-out RenderFrameHost concept by deleting the RenderFrameHost for the main frame on cross-process navigations and only keeping a RenderFrameProxyHost around. It is doing this for --site-per-process only, so all details can be fleshed out. Subsequent CL will remove the --site-per-process restriction and enable it in all modes of Chrome.

BUG= 357747 

Review URL: https://codereview.chromium.org/1142123002

Cr-Commit-Position: refs/heads/master@{#333100}

[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/chrome/browser/ui/zoom/zoom_controller_unittest.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/chrome/renderer/content_settings_observer.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/components/visitedlink/test/visitedlink_unittest.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/browser/frame_host/frame_tree.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/browser/frame_host/interstitial_page_impl.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/browser/frame_host/navigation_controller_impl_unittest.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/browser/frame_host/navigator_impl_unittest.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/browser/frame_host/render_frame_host_manager.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/browser/frame_host/render_frame_host_manager.h
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/browser/frame_host/render_frame_host_manager_unittest.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/browser/renderer_host/render_view_host_impl.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/browser/renderer_host/render_view_host_impl.h
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/browser/site_per_process_browsertest.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/browser/web_contents/web_contents_impl.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/browser/web_contents/web_contents_impl.h
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/public/test/test_renderer_host.h
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/renderer/accessibility/renderer_accessibility_browsertest.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/renderer/history_entry.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/renderer/render_frame_impl.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/renderer/render_frame_proxy.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/renderer/render_view_browsertest.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/renderer/render_view_impl.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/test/test_render_frame_host.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/test/test_render_view_host.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/test/test_render_view_host.h
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/test/test_web_contents.cc
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/content/test/test_web_contents.h
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/testing/buildbot/chromium.fyi.json
[modify] http://crrev.com/4c0feb652441719f6410ac137ef42b51c7c14a6e/ui/views/controls/webview/webview_interactive_uitest.cc

Comment 70 by marshall@chromium.org, Jun 8 2015

I'm hitting the `DCHECK_IMPLIES(is_live, render_view_host_->IsRenderViewLive())` assertion in RenderFrameHostImpl::IsRenderFrameLive() when calling NavigationController::LoadURL() from WebContentsDelegate::WebContentsCreated() for a new popup window. Using master revision 14bd12d6 (#333041) with a 32-bit Debug build on a Content API-based application on Windows 8.1 64-bit. I'm not specifying any command-line flags related to site isolation. Any suggestions on how to resolve this? Thanks.

Call stack:

>	libcef.dll!base::debug::BreakDebugger() Line 21	C++
 	libcef.dll!logging::LogMessage::~LogMessage() Line 642	C++
 	libcef.dll!content::RenderFrameHostImpl::IsRenderFrameLive() Line 664	C++
 	libcef.dll!content::RenderFrameHostManager::UpdateStateForNavigate(const GURL & dest_url, content::SiteInstance * source_instance, content::SiteInstance * dest_instance, ui::PageTransition transition, bool dest_is_restore, bool dest_is_view_source_mode, const content::GlobalRequestID & transferred_request_id, int bindings) Line 1946	C++
 	libcef.dll!content::RenderFrameHostManager::Navigate(const content::NavigationEntryImpl & entry) Line 194	C++
 	libcef.dll!content::NavigatorImpl::NavigateToEntry(content::FrameTreeNode * frame_tree_node, const content::NavigationEntryImpl & entry, content::NavigationController::ReloadType reload_type) Line 261	C++
 	libcef.dll!content::NavigatorImpl::NavigateToPendingEntry(content::FrameTreeNode * frame_tree_node, content::NavigationController::ReloadType reload_type) Line 336	C++
 	libcef.dll!content::WebContentsImpl::NavigateToPendingEntry(content::NavigationController::ReloadType reload_type) Line 2016	C++
 	libcef.dll!content::NavigationControllerImpl::NavigateToPendingEntry(content::NavigationController::ReloadType reload_type) Line 1867	C++
 	libcef.dll!content::NavigationControllerImpl::LoadEntry(content::NavigationEntryImpl * entry) Line 436	C++
 	libcef.dll!content::NavigationControllerImpl::LoadURLWithParams(const content::NavigationController::LoadURLParams & params) Line 792	C++
 	libcef.dll!content::NavigationControllerImpl::LoadURL(const GURL & url, const content::Referrer & referrer, ui::PageTransition transition, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & extra_headers) Line 669	C++
 	libcef.dll!CefBrowserHostImpl::LoadURL(__int64 frame_id, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & url, const content::Referrer & referrer, ui::PageTransition transition, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & extra_headers) Line 1702	C++
 	libcef.dll!CefFrameHostImpl::LoadURL(const CefStringBase<CefStringTraitsUTF16> & url) Line 140	C++
 	libcef.dll!`anonymous namespace'::frame_load_url(_cef_frame_t * self, const _cef_string_utf16_t * url) Line 192	C++
 	cef_unittests.exe!CefFrameCToCpp::LoadURL(const CefStringBase<CefStringTraitsUTF16> & url) Line 190	C++
 	cef_unittests.exe!`anonymous namespace'::PopupNavTestHandler::OnAfterCreated(CefRefPtr<CefBrowser> browser) Line 2061	C++
 	cef_unittests.exe!`anonymous namespace'::life_span_handler_on_after_created(_cef_life_span_handler_t * self, _cef_browser_t * browser) Line 139	C++
 	libcef.dll!CefLifeSpanHandlerCToCpp::OnAfterCreated(CefRefPtr<CefBrowser> browser) Line 100	C++
 	libcef.dll!CefBrowserHostImpl::CreateInternal(const CefWindowInfo & window_info, const CefStructBase<CefBrowserSettingsTraits> & settings, CefRefPtr<CefClient> client, content::WebContents * web_contents, scoped_refptr<CefBrowserInfo> browser_info, HWND__ * opener, CefRefPtr<CefRequestContext> request_context) Line 496	C++
 	libcef.dll!CefBrowserHostImpl::WebContentsCreated(content::WebContents * source_contents, int opener_render_frame_id, const std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > & frame_name, const GURL & target_url, content::WebContents * new_contents) Line 2425	C++
 	libcef.dll!content::WebContentsImpl::CreateNewWindow(int render_process_id, int route_id, int main_frame_route_id, const ViewHostMsg_CreateWindow_Params & params, content::SessionStorageNamespace * session_storage_namespace) Line 1666	C++
 	libcef.dll!content::RenderViewHostImpl::CreateNewWindow(int route_id, int main_frame_route_id, const ViewHostMsg_CreateWindow_Params & params, content::SessionStorageNamespace * session_storage_namespace) Line 994	C++
 	libcef.dll!content::RenderWidgetHelper::OnCreateWindowOnUI(const ViewHostMsg_CreateWindow_Params & params, int route_id, int main_frame_route_id, content::SessionStorageNamespace * session_storage_namespace) Line 150	C++
 	libcef.dll!base::internal::RunnableAdapter<void (__thiscall content::RenderWidgetHelper::*)(ViewHostMsg_CreateWindow_Params const &,int,int,content::SessionStorageNamespace *)>::Run(content::RenderWidgetHelper * object, const ViewHostMsg_CreateWindow_Params & <args_0>, const int & <args_1>, const int & <args_2>, content::SessionStorageNamespace * const & <args_3>) Line 176	C++
 	libcef.dll!base::internal::InvokeHelper<0,void,base::internal::RunnableAdapter<void (__thiscall content::RenderWidgetHelper::*)(ViewHostMsg_CreateWindow_Params const &,int,int,content::SessionStorageNamespace *)>,base::internal::TypeList<content::RenderWidgetHelper * const &,ViewHostMsg_CreateWindow_Params const &,int const &,int const &,content::SessionStorageNamespace *> >::MakeItSo(base::internal::RunnableAdapter<void (__thiscall content::RenderWidgetHelper::*)(ViewHostMsg_CreateWindow_Params const &,int,int,content::SessionStorageNamespace *)> runnable, content::RenderWidgetHelper * const & <args_0>, const ViewHostMsg_CreateWindow_Params & <args_1>, const int & <args_2>, const int & <args_3>, content::SessionStorageNamespace * <args_4>) Line 294	C++
 	libcef.dll!base::internal::Invoker<base::IndexSequence<0,1,2,3,4>,base::internal::BindState<base::internal::RunnableAdapter<void (__thiscall content::RenderWidgetHelper::*)(ViewHostMsg_CreateWindow_Params const &,int,int,content::SessionStorageNamespace *)>,void __cdecl(content::RenderWidgetHelper *,ViewHostMsg_CreateWindow_Params const &,int,int,content::SessionStorageNamespace *),base::internal::TypeList<content::RenderWidgetHelper *,ViewHostMsg_CreateWindow_Params,int,int,scoped_refptr<content::SessionStorageNamespace> > >,base::internal::TypeList<base::internal::UnwrapTraits<content::RenderWidgetHelper *>,base::internal::UnwrapTraits<ViewHostMsg_CreateWindow_Params>,base::internal::UnwrapTraits<int>,base::internal::UnwrapTraits<int>,base::internal::UnwrapTraits<scoped_refptr<content::SessionStorageNamespace> > >,base::internal::InvokeHelper<0,void,base::internal::RunnableAdapter<void (__thiscall content::RenderWidgetHelper::*)(ViewHostMsg_CreateWindow_Params const &,int,int,content::SessionStorageNamespace *)>,base::internal::TypeList<content::RenderWidgetHelper * const &,ViewHostMsg_CreateWindow_Params const &,int const &,int const &,content::SessionStorageNamespace *> >,void __cdecl(void)>::Run(base::internal::BindStateBase * base) Line 346	C++
 	libcef.dll!base::Callback<void __cdecl(void)>::Run() Line 396	C++
 	libcef.dll!base::debug::TaskAnnotator::RunTask(const char * queue_function, const char * run_function, const base::PendingTask & pending_task) Line 64	C++
 	libcef.dll!base::MessageLoop::RunTask(const base::PendingTask & pending_task) Line 455	C++
 	libcef.dll!base::MessageLoop::DeferOrRunPendingTask(const base::PendingTask & pending_task) Line 465	C++
 	libcef.dll!base::MessageLoop::DoWork() Line 574	C++
 	libcef.dll!base::MessagePumpForUI::DoRunLoop() Line 184	C++
 	libcef.dll!base::MessagePumpWin::RunWithDispatcher(base::MessagePump::Delegate * delegate, base::MessagePumpDispatcher * dispatcher) Line 51	C++
 	libcef.dll!base::MessagePumpWin::Run(base::MessagePump::Delegate * delegate) Line 58	C++
 	libcef.dll!base::MessageLoop::RunHandler() Line 418	C++
 	libcef.dll!base::RunLoop::Run() Line 56	C++
 	libcef.dll!base::MessageLoop::Run() Line 281	C++
 	libcef.dll!CefBrowserMessageLoop::RunMessageLoop() Line 28	C++
 	libcef.dll!CefRunMessageLoop() Line 178	C++
 	libcef.dll!cef_run_message_loop() Line 297	C++
 	cef_unittests.exe!CefRunMessageLoop() Line 288	C++
 	cef_unittests.exe!main(int argc, char * * argv) Line 183	C++

Comment 71 by nasko@chromium.org, Jun 8 2015

Since this is a popup test that creates a new window, I'm guessing it is due to  issue 492830 , which has a fix in progress - https://codereview.chromium.org/1159143008/. You can try to verify if the proposed patch will fix this CHECK for you and if you do, please let us know.

Comment 72 by lfg@chromium.org, Jun 8 2015

@marshall: Can you check if https://codereview.chromium.org/1159143008/ fixes your issue?

Comment 73 by marshall@chromium.org, Jun 8 2015

@#71-72: Thanks, that change set fixed the crash for me :)

Comment 74 by bugdroid1@chromium.org, Jun 26 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea32ae1fb0617e83b96b965ba10a31df38aa3269

commit ea32ae1fb0617e83b96b965ba10a31df38aa3269
Author: nasko <nasko@chromium.org>
Date: Fri Jun 26 15:16:02 2015

Pass explicitly the frame to HistoryController::GoToEntry

When doing a session history navigation cross-process in a frame which
already has a proxy, the proxy is replaced by a new real frame. In this
case, HistoryController doesn't know about the new frame it needs to
interact with, so this CL adds it as an explicit parameter in GoToEntry.

This scenario becomes possible once we stop using swapped out frames
for navigation in regular Chrome.

BUG= 357747 

Review URL: https://codereview.chromium.org/1212363002

Cr-Commit-Position: refs/heads/master@{#336370}

[modify] http://crrev.com/ea32ae1fb0617e83b96b965ba10a31df38aa3269/content/renderer/history_controller.cc
[modify] http://crrev.com/ea32ae1fb0617e83b96b965ba10a31df38aa3269/content/renderer/history_controller.h
[modify] http://crrev.com/ea32ae1fb0617e83b96b965ba10a31df38aa3269/content/renderer/render_frame_impl.cc

Comment 76 by bugdroid1@chromium.org, Jul 1 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d13d3940db59fd6fc93430acad64d081812e575d

commit d13d3940db59fd6fc93430acad64d081812e575d
Author: nasko <nasko@chromium.org>
Date: Wed Jul 01 14:09:46 2015

Enable AllowTargetedNavigationsAfterSwap and DisownOpener.

A few fixes have now landed making these two test pass with --site-per-process.

TBR=jam@chromium.org
BUG= 496923 , 357747 

Review URL: https://codereview.chromium.org/1212243003

Cr-Commit-Position: refs/heads/master@{#337018}

[modify] http://crrev.com/d13d3940db59fd6fc93430acad64d081812e575d/testing/buildbot/chromium.fyi.json

Comment 77 by bugdroid1@chromium.org, Aug 20 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5fb985beb21d826d9027e534550fb1be3961eba5

commit 5fb985beb21d826d9027e534550fb1be3961eba5
Author: nasko <nasko@chromium.org>
Date: Thu Aug 20 17:28:16 2015

Prepare browser plugin code for swapped out removal.

When swapped out state is removed, there will be no longer creation of
swapped out RenderViews, which browser plugin relies on. This CL makes
the codepath conditional on the IsSwappedOutStateForbidden() result and
makes the switch away from swapped out much cleaner.

BUG= 357747 

Review URL: https://codereview.chromium.org/1299323003

Cr-Commit-Position: refs/heads/master@{#344528}

[modify] http://crrev.com/5fb985beb21d826d9027e534550fb1be3961eba5/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] http://crrev.com/5fb985beb21d826d9027e534550fb1be3961eba5/content/browser/web_contents/web_contents_impl.cc

Comment 78 by laforge@google.com, Aug 24 2015

Labels: Pri-2
Adding default Pri-2

Comment 79 by laforge@google.com, Aug 24 2015

Adding default Pri-2

Comment 80 by laforge@google.com, Aug 24 2015

Adding default Pri-2

Comment 81 by laforge@google.com, Aug 24 2015

Adding default Pri-2

Comment 82 by bugdroid1@chromium.org, Sep 11 2015

Project Member
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=202163

------------------------------------------------------------------
r202163 | nasko@chromium.org | 2015-09-11T20:51:16.610369Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebRemoteFrameImpl.cpp?r1=202163&r2=202162&pathrev=202163
   M http://src.chromium.org/viewvc/blink/trunk/public/web/WebRemoteFrame.h?r1=202163&r2=202162&pathrev=202163
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebRemoteFrameImpl.h?r1=202163&r2=202162&pathrev=202163

The current implementation of <webview> relies on being able to access the V8 
script context of the swapped out RenderFrame representing the frame in the 
remote process.

When swapped out is disabled, there is no longer a RenderFrame, but we have
RenderFrameProxy, which is backed by WebRemoteFrame. As such, to avoid breaking
<webview>, it must be possible to get the V8 script context from a WebRemoteFrame.
This CL adds such a method, which should not be used by any other code. Once the
full migration of <webview> from BrowserPlugin to out-of-process iframes is
complete, we can remove this method.

BUG= 357747 

Review URL: https://codereview.chromium.org/1331213002
-----------------------------------------------------------------

Comment 83 by bugdroid1@chromium.org, Sep 14 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/89aa05ab8d8a42d2587e5ad3cc6ac245519ce588

commit 89aa05ab8d8a42d2587e5ad3cc6ac245519ce588
Author: nasko <nasko@chromium.org>
Date: Mon Sep 14 18:33:40 2015

Prepare GuestView for removing swapped out RenderFrame.

GuestView code is dependent on the usage of swapped out RenderFrame for
the out-of-process component. Since swapped out state is going away,
this CL removes this dependency.

BUG= 357747 

Review URL: https://codereview.chromium.org/1337283003

Cr-Commit-Position: refs/heads/master@{#348662}

[modify] http://crrev.com/89aa05ab8d8a42d2587e5ad3cc6ac245519ce588/components/guest_view/renderer/guest_view_request.cc
[modify] http://crrev.com/89aa05ab8d8a42d2587e5ad3cc6ac245519ce588/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc

Comment 84 by bugdroid1@chromium.org, Sep 14 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c749f3a0fad10de6b56d49351f9b99907f16750

commit 5c749f3a0fad10de6b56d49351f9b99907f16750
Author: nasko <nasko@chromium.org>
Date: Mon Sep 14 18:39:41 2015

Prepare MimeHanderViewContainer for removing swapped out RenderFrame.

The current implementation of MimeHanderViewContainer is dependent on
the usage of swapped out RenderFrame for the out-of-process component.
Since swapped out state is going away, this CL removes this dependency.

Credit goes to dcheng@, who found the simplest solution.

BUG= 357747 

Review URL: https://codereview.chromium.org/1335023004

Cr-Commit-Position: refs/heads/master@{#348666}

[modify] http://crrev.com/5c749f3a0fad10de6b56d49351f9b99907f16750/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc

Comment 85 by bugdroid1@chromium.org, Sep 14 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/56cd49d9f09779ac96a4885c09e3d9697c5c15bf

commit 56cd49d9f09779ac96a4885c09e3d9697c5c15bf
Author: nasko <nasko@chromium.org>
Date: Mon Sep 14 19:51:49 2015

Prepare RFHM for disabling swapped out state.

This CL ensures that RenderFrameHostManager doesn't create swapped out
RenderViewHosts when swapped out state is disabled. It should be creating
proxy objects instead.

BUG= 357747 

Review URL: https://codereview.chromium.org/1341003002

Cr-Commit-Position: refs/heads/master@{#348688}

[modify] http://crrev.com/56cd49d9f09779ac96a4885c09e3d9697c5c15bf/content/browser/frame_host/render_frame_host_manager.cc

Comment 86 by bugdroid1@chromium.org, Sep 28 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6bdf645df32dbfa0ff4ae4925468519800f74827

commit 6bdf645df32dbfa0ff4ae4925468519800f74827
Author: nasko <nasko@chromium.org>
Date: Mon Sep 28 23:45:09 2015

TextAutosizer should handle frames without a FrameView.

During cross-process navigation, it is possible to create a LocalFrame
that doesn't have a FrameView, since the latter gets created at the time
a document commits. Since the commit in this case happens later in time,
the current code crashes as it expects the view to be there.

BUG= 357747 

Review URL: https://codereview.chromium.org/1374783005

Cr-Commit-Position: refs/heads/master@{#351204}

[modify] http://crrev.com/6bdf645df32dbfa0ff4ae4925468519800f74827/third_party/WebKit/Source/core/layout/TextAutosizer.cpp

Comment 87 by bugdroid1@chromium.org, Sep 30 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b8cc9ba3b9d0b8478bfd5d63130dfcfc69fbe6e6

commit b8cc9ba3b9d0b8478bfd5d63130dfcfc69fbe6e6
Author: nasko <nasko@chromium.org>
Date: Wed Sep 30 00:17:48 2015

Disable support for swapped out RenderFrame(Host) on desktop.

This CL disables the usage of swapped out RenderFrame(Host) objects. Instead
RenderFrameProxy(Host) is used. Disabling is done through setting a
boolean value in checking for swapped out support and the actual code
will be removed in follow up CLs, once this one sticks and no issues are found.

Android is left alone, as troubleshooting why content_browsertests is failing
on the bots is very time consuming. Follow up patch will disable it there too.

BUG= 357747 

Review URL: https://codereview.chromium.org/1199313006

Cr-Commit-Position: refs/heads/master@{#351446}

[modify] http://crrev.com/b8cc9ba3b9d0b8478bfd5d63130dfcfc69fbe6e6/content/common/site_isolation_policy.cc

Comment 88 by bugdroid1@chromium.org, Sep 30 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e8a98e91d731af4c74f7e933727517fa2b924f87

commit e8a98e91d731af4c74f7e933727517fa2b924f87
Author: zhaoqin <zhaoqin@chromium.org>
Date: Wed Sep 30 18:13:45 2015

Revert of Disable support for swapped out RenderFrame(Host) on desktop. (patchset #16 id:300001 of https://codereview.chromium.org/1199313006/ )

Reason for revert:
This CL is the culprit of RenderFrameHostManagerTest.NoScriptAccessAfterSwapOut content_browsertests failure on TSan bots

BUG=537689

Original issue's description:
> Disable support for swapped out RenderFrame(Host) on desktop.
>
> This CL disables the usage of swapped out RenderFrame(Host) objects. Instead
> RenderFrameProxy(Host) is used. Disabling is done through setting a
> boolean value in checking for swapped out support and the actual code
> will be removed in follow up CLs, once this one sticks and no issues are found.
>
> Android is left alone, as troubleshooting why content_browsertests is failing
> on the bots is very time consuming. Follow up patch will disable it there too.
>
> BUG= 357747 
>
> Committed: https://crrev.com/b8cc9ba3b9d0b8478bfd5d63130dfcfc69fbe6e6
> Cr-Commit-Position: refs/heads/master@{#351446}

TBR=creis@chromium.org,nasko@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 357747 

Review URL: https://codereview.chromium.org/1374543004

Cr-Commit-Position: refs/heads/master@{#351597}

[modify] http://crrev.com/e8a98e91d731af4c74f7e933727517fa2b924f87/content/common/site_isolation_policy.cc

Comment 90 by bugdroid1@chromium.org, Oct 1 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/555e2bb727ad24696f7c5eb8e037b86758c898a6

commit 555e2bb727ad24696f7c5eb8e037b86758c898a6
Author: nasko <nasko@chromium.org>
Date: Thu Oct 01 22:45:40 2015

Move RFHM browser tests to the embedded test server.

Using the spawned test server leads to flaky tests on Android, as test
server startup in a separate process takes up quite a bit of time. This
CL moves most tests away from this setup to use the embedded test server,
which runs in-process and doesn't have such a startup cost.

BUG= 357747 , 418236

Review URL: https://codereview.chromium.org/1378403002

Cr-Commit-Position: refs/heads/master@{#351910}

[modify] http://crrev.com/555e2bb727ad24696f7c5eb8e037b86758c898a6/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] http://crrev.com/555e2bb727ad24696f7c5eb8e037b86758c898a6/content/test/data/click-noreferrer-links.html

Comment 91 by lafo...@chromium.org, Oct 1 2015

Labels: Hotlist-Recharge
This issue likely requires triage.  The current issue owner may be inactive (i.e. hasn't fixed an issue in the last 30 days or commented in this particular issue in the last 90 days).  Thanks for helping out!

-Anthony

Comment 92 by bugdroid1@chromium.org, Oct 3 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/885b6b65af2d7504cdda6091516d24704d2400b3

commit 885b6b65af2d7504cdda6091516d24704d2400b3
Author: nasko <nasko@chromium.org>
Date: Sat Oct 03 06:57:30 2015

TextAutosizer should handle cross-process navigation and RemoteFrame.

In https://codereview.chromium.org/1374783005/ the fix tested for whether
the frame has a FrameView or not. The problem turns out to be a bit
different - the check for RemoteFrame and the mainFrame do not point
to the same object, which leaves the code still open to problems.

This CL is ensuring that both the check for RemoteFrame and the usage of
the main LocalFrame point to the same object.

BUG= 357747 

Review URL: https://codereview.chromium.org/1375613004

Cr-Commit-Position: refs/heads/master@{#352249}

[modify] http://crrev.com/885b6b65af2d7504cdda6091516d24704d2400b3/third_party/WebKit/Source/core/layout/TextAutosizer.cpp

Comment 93 by nasko@google.com, Oct 5 2015

Labels: -Hotlist-Recharge
Removing Hotlist-Recharge, as this is long-term work that is close to completion, but not done yet.

Comment 94 by bugdroid1@chromium.org, Oct 8 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aa836bae06656808d56e0afde4944b6dcca97a10

commit aa836bae06656808d56e0afde4944b6dcca97a10
Author: nasko <nasko@chromium.org>
Date: Thu Oct 08 14:30:44 2015

Disable support for swapped out RenderFrame(Host)

Another attempt at disabling swapped out. The crashes and bugs found during the first attempt should be fixed now.

BUG= 357747 

Review URL: https://codereview.chromium.org/1381443003

Cr-Commit-Position: refs/heads/master@{#353055}

[modify] http://crrev.com/aa836bae06656808d56e0afde4944b6dcca97a10/content/common/site_isolation_policy.cc

Comment 95 by bugdroid1@chromium.org, Oct 10 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2f50210f15adbfde970bbfaeba6271d71e3fce32

commit 2f50210f15adbfde970bbfaeba6271d71e3fce32
Author: nasko <nasko@chromium.org>
Date: Sat Oct 10 00:38:23 2015

Revert of Disable support for swapped out RenderFrame(Host) (patchset #6 id:120001 of https://codereview.chromium.org/1381443003/ )

Reason for revert:
Speculative revert to see if crashes hit in crbug.com/541578 will disappear.

Original issue's description:
> Disable support for swapped out RenderFrame(Host)
>
> Another attempt at disabling swapped out. The crashes and bugs found during the first attempt should be fixed now.
>
> BUG= 357747 
>
> Committed: https://crrev.com/aa836bae06656808d56e0afde4944b6dcca97a10
> Cr-Commit-Position: refs/heads/master@{#353055}

TBR=creis@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 357747 

Review URL: https://codereview.chromium.org/1394373003

Cr-Commit-Position: refs/heads/master@{#353434}

[modify] http://crrev.com/2f50210f15adbfde970bbfaeba6271d71e3fce32/content/common/site_isolation_policy.cc

Comment 96 by bugdroid1@chromium.org, Oct 13 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/144cbce186be2afe854b7c5d9ba6780fbaea07b8

commit 144cbce186be2afe854b7c5d9ba6780fbaea07b8
Author: nasko <nasko@chromium.org>
Date: Tue Oct 13 20:58:14 2015

Disable support for swapped out RenderFrame(Host) and add instrumentation

This CL is aiming to collect information about cases where the
FocusController's focused frame is set to a RemoteFrame. This is the root
cause of the crash in https://crbug.com/541578.

BUG=541578,  357747 

Review URL: https://codereview.chromium.org/1397323004

Cr-Commit-Position: refs/heads/master@{#353847}

[modify] http://crrev.com/144cbce186be2afe854b7c5d9ba6780fbaea07b8/content/common/site_isolation_policy.cc
[modify] http://crrev.com/144cbce186be2afe854b7c5d9ba6780fbaea07b8/content/renderer/render_widget.cc

Comment 97 by bugdroid1@chromium.org, Oct 14 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7155b70cdb939ebb07013eebfeaec23ad3ce2f15

commit 7155b70cdb939ebb07013eebfeaec23ad3ce2f15
Author: nasko <nasko@chromium.org>
Date: Wed Oct 14 13:39:42 2015

Revert of Disable support for swapped out RenderFrame(Host) and add instrumentation (patchset #2 id:20001 of https://codereview.chromium.org/1397323004/ )

Reason for revert:
There are enough dumps at this time and this CL is no longer needed.

Original issue's description:
> Disable support for swapped out RenderFrame(Host) and add instrumentation
>
> This CL is aiming to collect information about cases where the
> FocusController's focused frame is set to a RemoteFrame. This is the root
> cause of the crash in https://crbug.com/541578.
>
> BUG=541578,  357747 
>
> Committed: https://crrev.com/144cbce186be2afe854b7c5d9ba6780fbaea07b8
> Cr-Commit-Position: refs/heads/master@{#353847}

TBR=dcheng@chromium.org,rsesek@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=541578,  357747 

Review URL: https://codereview.chromium.org/1403613004

Cr-Commit-Position: refs/heads/master@{#354013}

[modify] http://crrev.com/7155b70cdb939ebb07013eebfeaec23ad3ce2f15/content/common/site_isolation_policy.cc
[modify] http://crrev.com/7155b70cdb939ebb07013eebfeaec23ad3ce2f15/content/renderer/render_widget.cc

Comment 98 by nasko@google.com, Oct 14 2015

Blockedon: chromium:543226

Comment 99 by bugdroid1@chromium.org, Oct 16 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c6edf7e60d1d5dd6202fd778dbac8605fe71036d

commit c6edf7e60d1d5dd6202fd778dbac8605fe71036d
Author: nasko <nasko@chromium.org>
Date: Fri Oct 16 01:48:45 2015

Ignore InputMsg IPCs if RenderView is in swapped out state.

InputMsg IPCs are dispatched to the compositor thread when they arrive
in the renderer process. If not handled there, they are sent back to the
main thread. This behavior allows for IPCs that are processed on the
main thread to come in order different than the browser process has sent
them in.

The crash in 541578 occurs when InputMsg_SetFocus is sent right before
FrameMsg_SwapOut. On the renderer side, the InputMsg_SetFocus message is
sent to the compositor thread and in the meantime the FrameMsg_SwapOut
message is processed on the main thread. After swapping out is complete
the top-level frame for the swapped out RenderView becomes a RemoteFrame
when swapped out RenderFrameHost usage is disabled. This causes the
FocusController to return RemoteFrame and that violates assumptions
elsewhere in code that all focused frames are LocalFrame.

Overall, swapped out RenderView should not be processing input messages
at all, so this CL implements this restriction.

BUG= 357747 , 541578

Review URL: https://codereview.chromium.org/1408873002

Cr-Commit-Position: refs/heads/master@{#354426}

[modify] http://crrev.com/c6edf7e60d1d5dd6202fd778dbac8605fe71036d/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] http://crrev.com/c6edf7e60d1d5dd6202fd778dbac8605fe71036d/content/renderer/render_view_impl.cc

Comment 100 by bugdroid1@chromium.org, Oct 17 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ae13c59638df3adf7c9a206f82680faf86d52310

commit ae13c59638df3adf7c9a206f82680faf86d52310
Author: nasko <nasko@chromium.org>
Date: Sat Oct 17 01:07:05 2015

Properly recreate swapped out RenderView.

This CL fixes how RenderView is recreated after a process crash. Due to not recreating the RenderFrameProxy in the case the RenderView is swapped out, the WebView ends up with no mainFrame() and crashes in various ways.

BUG= 357747 , 544271 

Review URL: https://codereview.chromium.org/1408743005

Cr-Commit-Position: refs/heads/master@{#354658}

[modify] http://crrev.com/ae13c59638df3adf7c9a206f82680faf86d52310/content/browser/frame_host/render_frame_host_manager.cc
[modify] http://crrev.com/ae13c59638df3adf7c9a206f82680faf86d52310/content/browser/frame_host/render_frame_host_manager.h
[modify] http://crrev.com/ae13c59638df3adf7c9a206f82680faf86d52310/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] http://crrev.com/ae13c59638df3adf7c9a206f82680faf86d52310/content/browser/renderer_host/render_view_host_impl.cc

Comment 101 by dcheng@chromium.org, Oct 17 2015

Cc: -site-isolation-dev@chromium.org
un-CCing site-isolation-dev so I don't get a double notification every time this is updated...

(If you want to explicitly follow updates, feel free to star/CC yourself)

Comment 102 by bugdroid1@chromium.org, Oct 17 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0b480ba36c61900d2941897bc74f42c8e70fd5cd

commit 0b480ba36c61900d2941897bc74f42c8e70fd5cd
Author: nasko <nasko@chromium.org>
Date: Sat Oct 17 02:49:46 2015

Disable support for swapped out RenderFrame(Host)

Attempt at disabling swapped out RenderFrame(Host) usage. Bugs fixed
since last attempt:

https://crbug.com/541578
https://crbug.com/543226
 https://crbug.com/544271 

BUG= 357747 

Review URL: https://codereview.chromium.org/1409773004

Cr-Commit-Position: refs/heads/master@{#354663}

[modify] http://crrev.com/0b480ba36c61900d2941897bc74f42c8e70fd5cd/content/common/site_isolation_policy.cc

Comment 103 by bugdroid1@chromium.org, Oct 19 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4dfcf99da1c72086f44018b5976e80801dd682a5

commit 4dfcf99da1c72086f44018b5976e80801dd682a5
Author: nasko <nasko@chromium.org>
Date: Mon Oct 19 20:57:18 2015

Revert of Disable support for swapped out RenderFrame(Host) (patchset #2 id:20001 of https://codereview.chromium.org/1409773004/ )

Reason for revert:
More bugs got exposed:

https://crbug.com/543896
https://crbug.com/544755
 https://crbug.com/544868 

Original issue's description:
> Disable support for swapped out RenderFrame(Host)
>
> Attempt at disabling swapped out RenderFrame(Host) usage. Bugs fixed
> since last attempt:
>
> https://crbug.com/541578
> https://crbug.com/543226
>  https://crbug.com/544271 
>
> BUG= 357747 
>
> Committed: https://crrev.com/0b480ba36c61900d2941897bc74f42c8e70fd5cd
> Cr-Commit-Position: refs/heads/master@{#354663}

TBR=creis@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 357747 

Review URL: https://codereview.chromium.org/1409373006

Cr-Commit-Position: refs/heads/master@{#354857}

[modify] http://crrev.com/4dfcf99da1c72086f44018b5976e80801dd682a5/content/common/site_isolation_policy.cc

Comment 104 by bugdroid1@chromium.org, Oct 19 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5f0b7462c229b8f79e13ab128917c130703627bc

commit 5f0b7462c229b8f79e13ab128917c130703627bc
Author: reillyg <reillyg@chromium.org>
Date: Mon Oct 19 22:01:09 2015

Revert of Properly recreate swapped out RenderView. (patchset #6 id:100001 of https://codereview.chromium.org/1408743005/ )

Reason for revert:
This appears to be causing a set of two memory leaks on the Webkit Linux Valgrind bots:

22,008 (48 direct, 21,960 indirect) bytes in 1 blocks are definitely lost in loss record 2,955 of 2,972
operator new(unsigned long) (m_replacemalloc/vg_replace_malloc.c:1140)
content::RenderProcessHostImpl::Init() (content/browser/renderer_host/render_process_host_impl.cc:710)
content::RenderFrameHostManager::InitRenderView(content::RenderViewHostImpl*, content::RenderFrameProxyHost*) (content/browser/frame_host/render_frame_host_manager.cc:1995)
content::RenderFrameHostManager::Navigate(GURL const&, content::FrameNavigationEntry const&, content::NavigationEntryImpl const&) (content/browser/frame_host/render_frame_host_manager.cc:421)
content::NavigatorImpl::NavigateToEntry(content::FrameTreeNode*, content::FrameNavigationEntry const&, content::NavigationEntryImpl const&, content::NavigationController::ReloadType, bool) (content/browser/frame_host/navigator_impl.cc:312)
content::NavigatorImpl::NavigateToPendingEntry(content::FrameTreeNode*, content::FrameNavigationEntry const&, content::NavigationController::ReloadType, bool) (/b/build/slave/webkit-rel-linux-valgrind-layout/build/src/out/Release/content_shell)
content::NavigationControllerImpl::NavigateToPendingEntryInternal(content::NavigationController::ReloadType) (content/browser/frame_host/navigation_controller_impl.cc:1785)
content::NavigationControllerImpl::NavigateToPendingEntry(content::NavigationController::ReloadType) (content/browser/frame_host/navigation_controller_impl.cc:1762)
content::NavigationControllerImpl::LoadEntry(scoped_ptr<content::NavigationEntryImpl, base::DefaultDeleter<content::NavigationEntryImpl> >) (content/browser/frame_host/navigation_controller_impl.cc:430)
content::NavigationControllerImpl::LoadURLWithParams(content::NavigationController::LoadURLParams const&) (content/browser/frame_host/navigation_controller_impl.cc:795)
content::Shell::LoadURLForFrame(GURL const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (content/shell/browser/shell.cc:190)
content::Shell::LoadURL(GURL const&) (content/shell/browser/shell.cc:182)
content::BlinkTestController::PrepareForLayoutTest(GURL const&, base::FilePath const&, bool, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (content/shell/browser/blink_test_controller.cc:274)
(anonymous namespace)::RunOneTest(test_runner::TestInfo const&, bool*, scoped_ptr<content::BrowserMainRunner, base::DefaultDeleter<content::BrowserMainRunner> > const&) (content/shell/browser/layout_test/layout_test_browser_main.cc:40)
(anonymous namespace)::RunTests(scoped_ptr<content::BrowserMainRunner, base::DefaultDeleter<content::BrowserMainRunner> > const&) (content/shell/browser/layout_test/layout_test_browser_main.cc:87)
LayoutTestBrowserMain(content::MainFunctionParams const&, scoped_ptr<content::BrowserMainRunner, base::DefaultDeleter<content::BrowserMainRunner> > const&) (content/shell/browser/layout_test/layout_test_browser_main.cc:141)
content::ShellMainDelegate::RunProcess(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, content::MainFunctionParams const&) (content/shell/app/shell_main_delegate.cc:271)
content::RunNamedProcessTypeMain(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) (content/app/content_main_runner.cc:365)
content::ContentMainRunnerImpl::Run() (content/app/content_main_runner.cc:798)
content::ContentMain(content::ContentMainParams const&) (content/app/content_main.cc:19)

48 (40 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 1,603 of 2,979
operator new(unsigned long) (m_replacemalloc/vg_replace_malloc.c:1140)
content::PushMessagingMessageFilter::PushMessagingMessageFilter(int, content::ServiceWorkerContextWrapper*) (content/browser/push_messaging/push_messaging_message_filter.cc:213)
content::RenderProcessHostImpl::CreateMessageFilters() (content/browser/renderer_host/render_process_host_impl.cc:939)
content::RenderProcessHostImpl::Init() (content/browser/renderer_host/render_process_host_impl.cc:661)
content::RenderFrameHostManager::InitRenderView(content::RenderViewHostImpl*, content::RenderFrameProxyHost*) (content/browser/frame_host/render_frame_host_manager.cc:1995)
content::RenderFrameHostManager::Navigate(GURL const&, content::FrameNavigationEntry const&, content::NavigationEntryImpl const&) (content/browser/frame_host/render_frame_host_manager.cc:421)
content::NavigatorImpl::NavigateToEntry(content::FrameTreeNode*, content::FrameNavigationEntry const&, content::NavigationEntryImpl const&, content::NavigationController::ReloadType, bool) (content/browser/frame_host/navigator_impl.cc:312)
content::NavigatorImpl::NavigateToPendingEntry(content::FrameTreeNode*, content::FrameNavigationEntry const&, content::NavigationController::ReloadType, bool) (/b/build/slave/webkit-rel-linux-valgrind-layout/build/src/out/Release/content_shell)
content::NavigationControllerImpl::NavigateToPendingEntryInternal(content::NavigationController::ReloadType) (content/browser/frame_host/navigation_controller_impl.cc:1785)
content::NavigationControllerImpl::NavigateToPendingEntry(content::NavigationController::ReloadType) (content/browser/frame_host/navigation_controller_impl.cc:1762)
content::NavigationControllerImpl::LoadEntry(scoped_ptr<content::NavigationEntryImpl, base::DefaultDeleter<content::NavigationEntryImpl> >) (content/browser/frame_host/navigation_controller_impl.cc:430)
content::NavigationControllerImpl::LoadURLWithParams(content::NavigationController::LoadURLParams const&) (content/browser/frame_host/navigation_controller_impl.cc:795)
content::Shell::LoadURLForFrame(GURL const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (content/shell/browser/shell.cc:190)
content::Shell::LoadURL(GURL const&) (content/shell/browser/shell.cc:182)
content::BlinkTestController::PrepareForLayoutTest(GURL const&, base::FilePath const&, bool, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (content/shell/browser/blink_test_controller.cc:274)
(anonymous namespace)::RunOneTest(test_runner::TestInfo const&, bool*, scoped_ptr<content::BrowserMainRunner, base::DefaultDeleter<content::BrowserMainRunner> > const&) (content/shell/browser/layout_test/layout_test_browser_main.cc:40)
(anonymous namespace)::RunTests(scoped_ptr<content::BrowserMainRunner, base::DefaultDeleter<content::BrowserMainRunner> > const&) (content/shell/browser/layout_test/layout_test_browser_main.cc:87)
LayoutTestBrowserMain(content::MainFunctionParams const&, scoped_ptr<content::BrowserMainRunner, base::DefaultDeleter<content::BrowserMainRunner> > const&) (content/shell/browser/layout_test/layout_test_browser_main.cc:141)
content::ShellMainDelegate::RunProcess(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, content::MainFunctionParams const&) (content/shell/app/shell_main_delegate.cc:271)
content::RunNamedProcessTypeMain(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) (content/app/content_main_runner.cc:365)
content::ContentMainRunnerImpl::Run() (content/app/content_main_runner.cc:798)
content::ContentMain(content::ContentMainParams const&) (content/app/content_main.cc:19)

Original issue's description:
> Properly recreate swapped out RenderView.
>
> This CL fixes how RenderView is recreated after a process crash. Due to not recreating the RenderFrameProxy in the case the RenderView is swapped out, the WebView ends up with no mainFrame() and crashes in various ways.
>
> BUG= 357747 , 544271 
>
> Committed: https://crrev.com/ae13c59638df3adf7c9a206f82680faf86d52310
> Cr-Commit-Position: refs/heads/master@{#354658}

TBR=creis@chromium.org,alexmos@chromium.org,nasko@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 357747 , 544271 

Review URL: https://codereview.chromium.org/1412333002

Cr-Commit-Position: refs/heads/master@{#354881}

[modify] http://crrev.com/5f0b7462c229b8f79e13ab128917c130703627bc/content/browser/frame_host/render_frame_host_manager.cc
[modify] http://crrev.com/5f0b7462c229b8f79e13ab128917c130703627bc/content/browser/frame_host/render_frame_host_manager.h
[modify] http://crrev.com/5f0b7462c229b8f79e13ab128917c130703627bc/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] http://crrev.com/5f0b7462c229b8f79e13ab128917c130703627bc/content/browser/renderer_host/render_view_host_impl.cc

Comment 105 by dcheng@chromium.org, Oct 20 2015

Blockedon: chromium:543949

Comment 106 by bugdroid1@chromium.org, Oct 22 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1f3ecb2eb8806026b81d6c5605aba11ea1d1859d

commit 1f3ecb2eb8806026b81d6c5605aba11ea1d1859d
Author: nasko <nasko@chromium.org>
Date: Thu Oct 22 17:30:05 2015

Refactor RFHM::InitRenderView to take in RenderFrameProxyHost parameter

This CL is picking only the refactoring part of https://codereview.chromium.org/1408743005/,
which contained change in behavior as well. It was reverted due to a
potential memory leak, so I am landing the refactoring and the actual
fix separately now.

BUG= 357747 ,  544271 

Review URL: https://codereview.chromium.org/1415303002

Cr-Commit-Position: refs/heads/master@{#355566}

[modify] http://crrev.com/1f3ecb2eb8806026b81d6c5605aba11ea1d1859d/content/browser/frame_host/render_frame_host_manager.cc
[modify] http://crrev.com/1f3ecb2eb8806026b81d6c5605aba11ea1d1859d/content/browser/frame_host/render_frame_host_manager.h

Comment 107 by bugdroid1@chromium.org, Oct 23 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2fd02526ca100834b7a2b13304203244670d7bb3

commit 2fd02526ca100834b7a2b13304203244670d7bb3
Author: nasko <nasko@chromium.org>
Date: Fri Oct 23 16:41:44 2015

Properly recreate swapped out RenderView.

This CL fixes how RenderView is recreated after a process crash. Due to not
recreating the RenderFrameProxy in the case the RenderView is swapped out, the
WebView ends up with no mainFrame() and crashes in various ways.

This is a reland of https://codereview.chromium.org/1408743005/ with the
refactoring portion committed separately in https://codereview.chromium.org/1415303002/.

BUG= 357747 ,  544271 

Review URL: https://codereview.chromium.org/1412173006

Cr-Commit-Position: refs/heads/master@{#355802}

[modify] http://crrev.com/2fd02526ca100834b7a2b13304203244670d7bb3/content/browser/frame_host/render_frame_host_manager.cc
[modify] http://crrev.com/2fd02526ca100834b7a2b13304203244670d7bb3/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] http://crrev.com/2fd02526ca100834b7a2b13304203244670d7bb3/content/browser/renderer_host/render_view_host_impl.cc

Comment 108 by creis@chromium.org, Nov 24 2015

Blockedon: chromium:545900

Comment 109 by creis@chromium.org, Nov 24 2015

Blockedon: chromium:548275

Comment 110 by nasko@chromium.org, Dec 4 2015

Blockedon: chromium:544868

Comment 111 by bugdroid1@chromium.org, Dec 16 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/346969918900aa4b42f9836e1cdc29ff53c43b7c

commit 346969918900aa4b42f9836e1cdc29ff53c43b7c
Author: nasko <nasko@chromium.org>
Date: Wed Dec 16 22:36:47 2015

Make WebContentsImpl::UpdateState navigation enties DCHECK conditional

When swapped out state is not used, a RenderViewHost is not guaranteed
to have a valid RenderFrameHost for the main frame. It is useful to
keep the DCHECK, but this CL makes it conditional on the presence of
a main RenderFrameHost.

BUG= 357747 

Review URL: https://codereview.chromium.org/1523163006

Cr-Commit-Position: refs/heads/master@{#365642}

[modify] http://crrev.com/346969918900aa4b42f9836e1cdc29ff53c43b7c/content/browser/web_contents/web_contents_impl.cc

Comment 112 by bugdroid1@chromium.org, Dec 17 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a590fefdefb1002bdeb4673fab4e88017cc3b211

commit a590fefdefb1002bdeb4673fab4e88017cc3b211
Author: nasko <nasko@chromium.org>
Date: Thu Dec 17 03:23:34 2015

Disable support for swapped out RenderFrame(Host)

Various bugs have been fixed since the last attempt so this is another
try at disabling swapped out RenderFrame(Host) usage.

BUG= 357747 

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/1529623003

Cr-Commit-Position: refs/heads/master@{#365711}

[modify] http://crrev.com/a590fefdefb1002bdeb4673fab4e88017cc3b211/content/common/site_isolation_policy.cc

Comment 113 by bugdroid1@chromium.org, Mar 12 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b3041b98a4757335bfa207a8b0c85c22bd277900

commit b3041b98a4757335bfa207a8b0c85c22bd277900
Author: nasko <nasko@chromium.org>
Date: Sat Mar 12 04:43:06 2016

Remove SiteIsolationPolicy::IsSwappedOutStateForbidden().

IsSwappedOutStateForbidden has been hardcoded to return true for the last
two milestones and it is now time to remove it.

BUG= 357747 ,  253537 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/1785153005

Cr-Commit-Position: refs/heads/master@{#380844}

[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/browser/frame_host/frame_tree.cc
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/browser/frame_host/navigator_impl_unittest.cc
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/browser/frame_host/render_frame_host_manager_unittest.cc
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/browser/renderer_host/render_view_host_impl.cc
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/common/site_isolation_policy.cc
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/common/site_isolation_policy.h
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/renderer/accessibility/renderer_accessibility_browsertest.cc
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/renderer/render_frame_proxy.cc
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/renderer/render_frame_proxy.h
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/b3041b98a4757335bfa207a8b0c85c22bd277900/content/renderer/render_view_impl.cc

Comment 116 by bugdroid1@chromium.org, Mar 17 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7ecc9a63ae68521e2189683fae86f5e89405986b

commit 7ecc9a63ae68521e2189683fae86f5e89405986b
Author: nasko <nasko@chromium.org>
Date: Thu Mar 17 00:22:22 2016

Remove swapped out state from RenderFrameHost.

Since swapped out RenderFrameHosts are no longer used, its swapped out
state is no longer necessary. This CL removes it along with a public
API that exposed it outside of content/.

BUG= 357747 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/1799163002

Cr-Commit-Position: refs/heads/master@{#381601}

[modify] https://crrev.com/7ecc9a63ae68521e2189683fae86f5e89405986b/chrome/browser/ssl/ssl_browser_tests.cc
[modify] https://crrev.com/7ecc9a63ae68521e2189683fae86f5e89405986b/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/7ecc9a63ae68521e2189683fae86f5e89405986b/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/7ecc9a63ae68521e2189683fae86f5e89405986b/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/7ecc9a63ae68521e2189683fae86f5e89405986b/content/browser/frame_host/render_frame_host_manager_unittest.cc
[modify] https://crrev.com/7ecc9a63ae68521e2189683fae86f5e89405986b/content/browser/renderer_host/render_view_host_impl.h
[modify] https://crrev.com/7ecc9a63ae68521e2189683fae86f5e89405986b/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/7ecc9a63ae68521e2189683fae86f5e89405986b/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/7ecc9a63ae68521e2189683fae86f5e89405986b/content/public/test/test_renderer_host.cc
[modify] https://crrev.com/7ecc9a63ae68521e2189683fae86f5e89405986b/content/public/test/test_renderer_host.h

Comment 118 by bugdroid1@chromium.org, Mar 22 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/80f7f60ce8e7c651c0d3d5957fbbb4ffe9972309

commit 80f7f60ce8e7c651c0d3d5957fbbb4ffe9972309
Author: lukasza <lukasza@chromium.org>
Date: Tue Mar 22 23:13:19 2016

More strict restrictions for content -> content/shell dependencies.

This CL removes content -> content/shell layering violation around
swapped-out filtering, by moving the filtering code into
LayoutTestContentClient in the appropriate layer.

This CL also tweaks content/DEPS to restricts allowed dependencies, by
expanding a comment into an actual DEPS rule that only allows content/shell
dependency for browser tests.

Additionally, a DEPS exception in content/common/DEPS is not needed anymore
(per https://codereview.chromium.org/1777503003/diff/20001/content/common/DEPS#newcode70).

BUG= 357747 ,  596736 

Review URL: https://codereview.chromium.org/1777503003

Cr-Commit-Position: refs/heads/master@{#382725}

[modify] https://crrev.com/80f7f60ce8e7c651c0d3d5957fbbb4ffe9972309/content/DEPS
[modify] https://crrev.com/80f7f60ce8e7c651c0d3d5957fbbb4ffe9972309/content/common/DEPS
[modify] https://crrev.com/80f7f60ce8e7c651c0d3d5957fbbb4ffe9972309/content/common/swapped_out_messages.cc
[modify] https://crrev.com/80f7f60ce8e7c651c0d3d5957fbbb4ffe9972309/content/content_shell.gypi
[modify] https://crrev.com/80f7f60ce8e7c651c0d3d5957fbbb4ffe9972309/content/shell/BUILD.gn
[modify] https://crrev.com/80f7f60ce8e7c651c0d3d5957fbbb4ffe9972309/content/shell/app/shell_main_delegate.cc
[modify] https://crrev.com/80f7f60ce8e7c651c0d3d5957fbbb4ffe9972309/content/shell/app/shell_main_delegate.h
[add] https://crrev.com/80f7f60ce8e7c651c0d3d5957fbbb4ffe9972309/content/shell/common/layout_test/layout_test_content_client.cc
[add] https://crrev.com/80f7f60ce8e7c651c0d3d5957fbbb4ffe9972309/content/shell/common/layout_test/layout_test_content_client.h

Comment 119 by bugdroid1@chromium.org, Mar 25 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a949e39cbe5f6ef8bad2e9221af8ee1152be2419

commit a949e39cbe5f6ef8bad2e9221af8ee1152be2419
Author: nasko <nasko@chromium.org>
Date: Fri Mar 25 22:18:44 2016

Remove unused code from RenderFrameProxyHost.

Since swapped out state on RenderFrameHost has been removed, there is
no longer usage of RenderFrameProxyHost transfering ownership and the
code can safely be removed.

BUG= 357747 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/1831233004

Cr-Commit-Position: refs/heads/master@{#383383}

[modify] https://crrev.com/a949e39cbe5f6ef8bad2e9221af8ee1152be2419/content/browser/frame_host/render_frame_proxy_host.cc
[modify] https://crrev.com/a949e39cbe5f6ef8bad2e9221af8ee1152be2419/content/browser/frame_host/render_frame_proxy_host.h

Comment 121 by bugdroid1@chromium.org, Mar 25 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/259799c180b8571c05473c0111f3e1fbf0e978ee

commit 259799c180b8571c05473c0111f3e1fbf0e978ee
Author: nasko <nasko@chromium.org>
Date: Fri Mar 25 23:17:04 2016

Remove CreateRenderFrameFlags enum.

The CreateRenderFrameFlags enum currently has only one value, which can
easily be replaced by a boolean. This CL is removing this enum in favor
of a bool value.

BUG= 357747 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/1833243002

Cr-Commit-Position: refs/heads/master@{#383401}

[modify] https://crrev.com/259799c180b8571c05473c0111f3e1fbf0e978ee/content/browser/frame_host/render_frame_host_factory.cc
[modify] https://crrev.com/259799c180b8571c05473c0111f3e1fbf0e978ee/content/browser/frame_host/render_frame_host_factory.h
[modify] https://crrev.com/259799c180b8571c05473c0111f3e1fbf0e978ee/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/259799c180b8571c05473c0111f3e1fbf0e978ee/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/259799c180b8571c05473c0111f3e1fbf0e978ee/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/259799c180b8571c05473c0111f3e1fbf0e978ee/content/browser/frame_host/render_frame_host_manager.h
[modify] https://crrev.com/259799c180b8571c05473c0111f3e1fbf0e978ee/content/test/test_render_frame_host_factory.cc
[modify] https://crrev.com/259799c180b8571c05473c0111f3e1fbf0e978ee/content/test/test_render_frame_host_factory.h

Comment 122 by bugdroid1@chromium.org, Apr 6 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e2ef58ac47ab6760814fb446007f95b1cffe225a

commit e2ef58ac47ab6760814fb446007f95b1cffe225a
Author: nasko <nasko@chromium.org>
Date: Wed Apr 06 09:44:24 2016

Remove RFHM::IsPendingDeletion

With RenderFrameHost always being deleted now, this method doesn't make
much sense. It is obsolete since all RenderFrameHosts are now deleted and
never reused, so the equivalent check is whether the RenderFrameHost is
in active state or not.

I've ran content_(browser|unit)tests with a CHECK inside
RenderFrameHostImpl::OnSwappedOut to ensure is always true and it passes clean.

BUG= 357747 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/1851253002

Cr-Commit-Position: refs/heads/master@{#385424}

[modify] https://crrev.com/e2ef58ac47ab6760814fb446007f95b1cffe225a/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/e2ef58ac47ab6760814fb446007f95b1cffe225a/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/e2ef58ac47ab6760814fb446007f95b1cffe225a/content/browser/frame_host/render_frame_host_manager.h
[modify] https://crrev.com/e2ef58ac47ab6760814fb446007f95b1cffe225a/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] https://crrev.com/e2ef58ac47ab6760814fb446007f95b1cffe225a/content/browser/frame_host/render_frame_host_manager_unittest.cc
[modify] https://crrev.com/e2ef58ac47ab6760814fb446007f95b1cffe225a/content/browser/site_per_process_browsertest.cc

Comment 123 by bugdroid1@chromium.org, Apr 8 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/19736cc9fd0f11697846b7ae43364a666b09a48c

commit 19736cc9fd0f11697846b7ae43364a666b09a48c
Author: nasko <nasko@chromium.org>
Date: Fri Apr 08 22:38:45 2016

Remove RenderFrameHostImplState and convert it to boolean.

Since RenderFrameHost no longer supports swapped out state, the state
enum RenderFrameHostImplState is no longer needed. It can be replaced by
a boolean indicating that it is waiting for swap-out ack from the renderer
process.

BUG= 357747 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/1849343004

Cr-Commit-Position: refs/heads/master@{#386232}

[modify] https://crrev.com/19736cc9fd0f11697846b7ae43364a666b09a48c/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/19736cc9fd0f11697846b7ae43364a666b09a48c/content/browser/frame_host/navigator_impl_unittest.cc
[modify] https://crrev.com/19736cc9fd0f11697846b7ae43364a666b09a48c/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/19736cc9fd0f11697846b7ae43364a666b09a48c/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/19736cc9fd0f11697846b7ae43364a666b09a48c/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] https://crrev.com/19736cc9fd0f11697846b7ae43364a666b09a48c/content/browser/frame_host/render_frame_host_manager_unittest.cc
[modify] https://crrev.com/19736cc9fd0f11697846b7ae43364a666b09a48c/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/19736cc9fd0f11697846b7ae43364a666b09a48c/content/browser/web_contents/web_contents_impl.cc

Comment 124 by bugdroid1@chromium.org, Apr 18 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e696e47994432cc1075f3fb74de7c7e9ac2b29a4

commit e696e47994432cc1075f3fb74de7c7e9ac2b29a4
Author: nasko <nasko@chromium.org>
Date: Mon Apr 18 23:45:58 2016

Remove RenderFrameHostManager::MoveToPendingDeleteHosts

This method is only called twice in one method after some code clean up
has happened. It seems we don't need a full method for that at this point.

BUG= 357747 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/1894193002

Cr-Commit-Position: refs/heads/master@{#388081}

[modify] https://crrev.com/e696e47994432cc1075f3fb74de7c7e9ac2b29a4/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/e696e47994432cc1075f3fb74de7c7e9ac2b29a4/content/browser/frame_host/render_frame_host_manager.h

Comment 125 by bugdroid1@chromium.org, May 2 2016

Project Member

Comment 127 by bugdroid1@chromium.org, May 17 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b4a1a32e9393518ebd63c52f021eb5142954c9d6

commit b4a1a32e9393518ebd63c52f021eb5142954c9d6
Author: dcheng <dcheng@chromium.org>
Date: Tue May 17 01:57:00 2016

✀ Remove postMessage plumbing for swappedout:// ✀

BUG= 357747 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/1973133002
Cr-Commit-Position: refs/heads/master@{#394016}

[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/components/test_runner/layout_test_runtime_flags.cc
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/components/test_runner/layout_test_runtime_flags.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/components/test_runner/test_runner.cc
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/components/test_runner/test_runner.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/components/test_runner/web_frame_test_client.cc
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/components/test_runner/web_frame_test_client.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/components/test_runner/web_frame_test_proxy.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/content/renderer/render_frame_impl.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/content/renderer/render_frame_proxy.cc
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/content/renderer/render_frame_proxy.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/content/renderer/render_view_browsertest.cc
[delete] https://crrev.com/2f6678c6f03f54be23988e925eb65539ec78b154/third_party/WebKit/LayoutTests/fast/events/intercept-postmessage-expected.txt
[delete] https://crrev.com/2f6678c6f03f54be23988e925eb65539ec78b154/third_party/WebKit/LayoutTests/fast/events/intercept-postmessage.html
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/core/frame/DOMWindow.cpp
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/core/frame/DOMWindow.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/core/frame/FrameClient.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/core/frame/LocalDOMWindow.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/core/frame/RemoteDOMWindow.cpp
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/core/frame/RemoteDOMWindow.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/core/frame/RemoteFrame.cpp
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/core/frame/RemoteFrame.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/core/frame/RemoteFrameClient.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/web/FrameLoaderClientImpl.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/web/RemoteFrameClientImpl.cpp
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/web/RemoteFrameClientImpl.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/Source/web/tests/FrameTestHelpers.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/public/web/WebFrameClient.h
[modify] https://crrev.com/b4a1a32e9393518ebd63c52f021eb5142954c9d6/third_party/WebKit/public/web/WebRemoteFrameClient.h

Comment 129 by jam@chromium.org, Oct 3 2016

Should this be marked as fixed?

Comment 130 by nasko@chromium.org, Oct 7 2016

Status: Fixed (was: Assigned)
I have one follow up nit item to fix, but I think it is fair to mark this as fixed and I'll land the fix at some point.

Yay! Done!

Comment 131 by bugdroid1@chromium.org, Mar 1 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f0b869613045ecabf265703452f1ae9fe4b3ee6

commit 9f0b869613045ecabf265703452f1ae9fe4b3ee6
Author: lfg <lfg@chromium.org>
Date: Wed Mar 01 18:18:48 2017

Remove unused member in RenderFrameHostImpl.

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

Review-Url: https://codereview.chromium.org/2722893002
Cr-Commit-Position: refs/heads/master@{#453974}

[modify] https://crrev.com/9f0b869613045ecabf265703452f1ae9fe4b3ee6/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/9f0b869613045ecabf265703452f1ae9fe4b3ee6/content/browser/frame_host/render_frame_host_impl.h

Comment 132 by bugdroid1@chromium.org, Aug 16 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/68e67c211adc3a15591978256009350a3fc50021

commit 68e67c211adc3a15591978256009350a3fc50021
Author: Charles Reis <creis@chromium.org>
Date: Wed Aug 16 02:02:03 2017

Remove stale references to the old internal swappedout:// URL.

BUG= 357747 
TEST=No behavior change.

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I6bb489fa53829eb10e0ad4a32dce888b807d40c9
Reviewed-on: https://chromium-review.googlesource.com/604395
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: Oystein Eftevaag <oysteine@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Charlie Reis (OOO Aug 17-24) <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494664}
[modify] https://crrev.com/68e67c211adc3a15591978256009350a3fc50021/chrome/browser/resource_coordinator/tab_manager.cc
[modify] https://crrev.com/68e67c211adc3a15591978256009350a3fc50021/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/68e67c211adc3a15591978256009350a3fc50021/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] https://crrev.com/68e67c211adc3a15591978256009350a3fc50021/content/browser/renderer_host/render_view_host_impl.h

Comment 133 by bugdroid1@chromium.org, Dec 15

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5defe939ef820964ab8bee47bbd26656a77a4689

commit 5defe939ef820964ab8bee47bbd26656a77a4689
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Sat Dec 15 03:02:04 2018

Remove a couple of stale comments about swapped-out RenderFrameHosts.

Bug:  357747 
Change-Id: I384b2f3e484d7cb330417662df56f6371797e0ce
Reviewed-on: https://chromium-review.googlesource.com/c/1374041
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616931}
[modify] https://crrev.com/5defe939ef820964ab8bee47bbd26656a77a4689/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/5defe939ef820964ab8bee47bbd26656a77a4689/content/browser/site_instance_impl.h
Showing comments 34 - 133 of 133 Older

Sign in to add a comment