New issue
Advanced search Search tips

Issue 901828 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 21
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

RC: Extension (and other) frame and process CUs are disconnected

Project Member Reported by siggi@chromium.org, Nov 5

Issue description

As-is, we only connect frame and process CUs on navigation, see https://cs.chromium.org/chromium/src/chrome/browser/resource_coordinator/tab_helper.cc?dr=CSs&g=0&l=127. This never happens for extension process frames.

It would seem reasonable to hook process and frame nodes together earlier, as there's a RenderProcessHost attached to every RenderFrameHost from creation.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/15fef9cc61f9ad53d8755651f0d40937c3f5d05e

commit 15fef9cc61f9ad53d8755651f0d40937c3f5d05e
Author: Sigurdur Asgeirsson <siggi@chromium.org>
Date: Fri Nov 16 22:29:05 2018

RC: Set Frame->Process association at RenderFrameHost creation.

At present this is only done at navigation time, which can leave frames
disconnected from their hosting process.

Bug:  901828 
Change-Id: I611eafebe5e7e54c046def38bbaf5b348dedfcb9
Reviewed-on: https://chromium-review.googlesource.com/c/1318405
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608994}
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/chrome/browser/resource_coordinator/tab_helper.cc
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/content/public/test/mock_render_process_host.cc
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/services/resource_coordinator/coordination_unit/coordination_unit_base.h
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.cc
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.cc
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.cc
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/services/resource_coordinator/coordination_unit/process_coordination_unit_impl_unittest.cc
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/services/resource_coordinator/observers/metrics_collector_unittest.cc
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/services/resource_coordinator/public/cpp/frame_resource_coordinator.cc
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/services/resource_coordinator/public/cpp/frame_resource_coordinator.h
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/services/resource_coordinator/public/cpp/page_resource_coordinator.cc
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/services/resource_coordinator/public/cpp/process_resource_coordinator.cc
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/services/resource_coordinator/public/cpp/process_resource_coordinator.h
[modify] https://crrev.com/15fef9cc61f9ad53d8755651f0d40937c3f5d05e/services/resource_coordinator/public/mojom/coordination_unit.mojom

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/78c35f24cb3094a2f7758f516a36644f6d1d6c3b

commit 78c35f24cb3094a2f7758f516a36644f6d1d6c3b
Author: Yuta Kitamura <yutak@chromium.org>
Date: Mon Nov 19 07:13:40 2018

Revert "RC: Set Frame->Process association at RenderFrameHost creation."

This reverts commit 15fef9cc61f9ad53d8755651f0d40937c3f5d05e.

Reason for revert: Likely introduced flaky assertion failure ( bug 906353 ).

Original change's description:
> RC: Set Frame->Process association at RenderFrameHost creation.
> 
> At present this is only done at navigation time, which can leave frames
> disconnected from their hosting process.
> 
> Bug:  901828 
> Change-Id: I611eafebe5e7e54c046def38bbaf5b348dedfcb9
> Reviewed-on: https://chromium-review.googlesource.com/c/1318405
> Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
> Reviewed-by: Will Harris <wfh@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#608994}

TBR=fdoray@chromium.org,wfh@chromium.org,jochen@chromium.org,siggi@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  901828 , 906353 
Change-Id: Idd7f4f1caf9a6ab884fd6de17ebb84b09e2c201e
Reviewed-on: https://chromium-review.googlesource.com/c/1341294
Reviewed-by: Yuta Kitamura <yutak@chromium.org>
Commit-Queue: Yuta Kitamura <yutak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609195}
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/chrome/browser/resource_coordinator/tab_helper.cc
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/content/public/test/mock_render_process_host.cc
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/services/resource_coordinator/coordination_unit/coordination_unit_base.h
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.cc
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.cc
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.cc
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/services/resource_coordinator/coordination_unit/process_coordination_unit_impl_unittest.cc
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/services/resource_coordinator/observers/metrics_collector_unittest.cc
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/services/resource_coordinator/public/cpp/frame_resource_coordinator.cc
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/services/resource_coordinator/public/cpp/frame_resource_coordinator.h
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/services/resource_coordinator/public/cpp/page_resource_coordinator.cc
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/services/resource_coordinator/public/cpp/process_resource_coordinator.cc
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/services/resource_coordinator/public/cpp/process_resource_coordinator.h
[modify] https://crrev.com/78c35f24cb3094a2f7758f516a36644f6d1d6c3b/services/resource_coordinator/public/mojom/coordination_unit.mojom

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 20

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

commit dfc286ca4b2e0caeebfcb62966adfb3ade665947
Author: Sigurdur Asgeirsson <siggi@chromium.org>
Date: Tue Nov 20 16:47:58 2018

Reland "RC: Set Frame->Process association at RenderFrameHost creation."

This is a reland of 15fef9cc61f9ad53d8755651f0d40937c3f5d05e

Original change's description:
> RC: Set Frame->Process association at RenderFrameHost creation.
>
> At present this is only done at navigation time, which can leave frames
> disconnected from their hosting process.
>
> Bug:  901828 
> Change-Id: I611eafebe5e7e54c046def38bbaf5b348dedfcb9
> Reviewed-on: https://chromium-review.googlesource.com/c/1318405
> Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
> Reviewed-by: Will Harris <wfh@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#608994}

TBR=wfh@chromium.org,jochen@chromium.org

Bug:  901828 
Change-Id: I7809f666e7cccba865d2220f9409936b727d1556
Reviewed-on: https://chromium-review.googlesource.com/c/1342699
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609727}
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/chrome/browser/resource_coordinator/tab_helper.cc
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/content/public/test/mock_render_process_host.cc
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/services/resource_coordinator/coordination_unit/coordination_unit_base.h
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.cc
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.cc
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.cc
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/services/resource_coordinator/coordination_unit/process_coordination_unit_impl_unittest.cc
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/services/resource_coordinator/observers/metrics_collector_unittest.cc
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/services/resource_coordinator/public/cpp/frame_resource_coordinator.cc
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/services/resource_coordinator/public/cpp/frame_resource_coordinator.h
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/services/resource_coordinator/public/cpp/page_resource_coordinator.cc
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/services/resource_coordinator/public/cpp/process_resource_coordinator.cc
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/services/resource_coordinator/public/cpp/process_resource_coordinator.h
[modify] https://crrev.com/dfc286ca4b2e0caeebfcb62966adfb3ade665947/services/resource_coordinator/public/mojom/coordination_unit.mojom

Status: Fixed (was: Assigned)
Looks like this one is going to stick.

Sign in to add a comment