Skip to content

Commit

Permalink
Revert "Reland "[code health] Remove host created notification use by…
Browse files Browse the repository at this point in the history
… RenderProcessHostTaskProvider""

This reverts commit 1974e89.

Reason for revert: Still causing crashes even with the prior fix.

Original change's description:
> Reland "[code health] Remove host created notification use by RenderProcessHostTaskProvider"
>
> This reverts commit 4f13745.
>
> Reason for revert: Crashes in the original CL are believed to have been fixed by https://crrev.com/c/4459636. See https://crbug.com/1414589#c8 for details.
>
> Original change's description:
> > Revert "[code health] Remove host created notification use by RenderProcessHostTaskProvider"
> >
> > This reverts commit d7c616f.
> >
> > Reason for revert: causing crashes on Windows and Mac
> >
> > Original change's description:
> > > [code health] Remove host created notification use by RenderProcessHostTaskProvider
> > >
> > > Removes the use of the render process host created notification
> > > in RenderProcessHostTaskProvider. Usage is replaced with the
> > > RenderProcessHostCreationObserver.
> > >
> > > Bug: 357627
> > > Change-Id: I5e64280ea824852e39a22a4e2eba6abbeb7f6fd1
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4182685
> > > Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
> > > Commit-Queue: Mike Wittman <wittman@chromium.org>
> > > Cr-Commit-Position: refs/heads/main@{#1103016}
> >
> > Bug: 357627, 1414589, 1414558, 1414580, 1414493, 1414630
> > Change-Id: Ifb2a661660b1c6a409043258b4381e116c662d33
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4237875
> > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> > Auto-Submit: Mike Wittman <wittman@chromium.org>
> > Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
> > Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1103629}
>
> Bug: 357627
> Change-Id: I27f7fac1152ee976b2452140904f1022b4a2f7ae
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4470947
> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
> Commit-Queue: Mike Wittman <wittman@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1135451}

(cherry picked from commit c231a59)

Bug: 357627, 1440114
Change-Id: Iadf2e94a213e69f0d3be17f835516fefe981dc5f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4477475
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Mike Wittman <wittman@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1136118}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4481181
Commit-Queue: Srinivas Sista <srinivassista@chromium.org>
Owners-Override: Srinivas Sista <srinivassista@chromium.org>
Reviewed-by: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/branch-heads/5735@{#8}
Cr-Branched-From: 2f562e4-refs/heads/main@{#1135570}
  • Loading branch information
Mike Wittman authored and Chromium LUCI CQ committed Apr 26, 2023
1 parent 34b29b2 commit 1d44ee1
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ void RenderProcessHostTaskProvider::StartUpdating() {
}
}

registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CREATED,
content::NotificationService::AllBrowserContextsAndSources());
registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
content::NotificationService::AllBrowserContextsAndSources());
registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
Expand Down Expand Up @@ -102,11 +104,6 @@ void RenderProcessHostTaskProvider::DeleteTask(
tasks_by_rph_id_.erase(itr);
}

void RenderProcessHostTaskProvider::OnRenderProcessHostCreated(
content::RenderProcessHost* host) {
CreateTask(host->GetID());
}

void RenderProcessHostTaskProvider::Observe(
int type,
const content::NotificationSource& source,
Expand All @@ -115,6 +112,9 @@ void RenderProcessHostTaskProvider::Observe(
content::Source<content::RenderProcessHost>(source).ptr();
ChildProcessData data(content::PROCESS_TYPE_RENDERER);
switch (type) {
case content::NOTIFICATION_RENDERER_PROCESS_CREATED:
CreateTask(host->GetID());
break;
case content::NOTIFICATION_RENDERER_PROCESS_CLOSED:
case content::NOTIFICATION_RENDERER_PROCESS_TERMINATED:
DeleteTask(host->GetID());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/render_process_host_creation_observer.h"

namespace task_manager {

Expand All @@ -22,10 +21,8 @@ class ChildProcessTask;
// This provides tasks that represent RenderProcessHost processes. It does so by
// listening to the notification service for the creation and destruction of the
// RenderProcessHost.
class RenderProcessHostTaskProvider
: public TaskProvider,
public content::RenderProcessHostCreationObserver,
public content::NotificationObserver {
class RenderProcessHostTaskProvider : public TaskProvider,
public content::NotificationObserver {
public:
RenderProcessHostTaskProvider();
RenderProcessHostTaskProvider(const RenderProcessHostTaskProvider&) = delete;
Expand All @@ -36,9 +33,6 @@ class RenderProcessHostTaskProvider
// task_manager::TaskProvider:
Task* GetTaskOfUrlRequest(int child_id, int route_id) override;

// content::RenderProcessHostCreationObserver:
void OnRenderProcessHostCreated(content::RenderProcessHost* host) override;

// content::NotificationObserver:
void Observe(int type,
const content::NotificationSource& source,
Expand Down

0 comments on commit 1d44ee1

Please sign in to comment.