Change EXTENSION_PROCESS_CREATED observers to use EXTENSION_HOST_CREATED.

Change EXTENSION_PROCESS_CREATED observers to use EXTENSION_HOST_CREATED.

Change EXTENSION_HOST_CREATED to be fired on RenderViewReady.
Remove the EXTENSION_PROCESS_CREATED notification.

This is related to fixing bug 71629, in particular, making the correct things appear in the task manager when the extension host reuses a process. This affects the new reload path (but only for unpacked extensions), but it's also visible when a process isn't created for a popup in lots-of-tabs situations.

The extension settings page (the other observer) already listens to enough notifications that it updates properly (too many of them, but that's another story -- see bug).

BUG=102950
TEST=TaskManagerBrowserTest.* and also: Open Task Manager. Open an extension popup, extension notification, and extension page in a tab; check that they all appear. Kill an extension with a background page and reload it from the notification bubble; check that it reappears.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=109163

[Yoyo Zhou, 2011-11-04 19:25:08]

mpcomplete: since we discussed this yesterday.
aa: since we discussed this today.

[Matt Perry, 2011-11-04 21:12:22]

lgtm

[Aaron Boodman, 2011-11-06 23:13:16]

http://codereview.chromium.org/8480007/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_process_manager.h (right):

http://codereview.chromium.org/8480007/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_process_manager.h:155: Profile* profile_;
Elsewhere in this class we get this from the browsing_context_. Is there a need
to store a reference to it?

[Yoyo Zhou, 2011-11-07 23:23:18]

FYI, with this change, I'm hitting a bug exposed by the task manager
browsertests, wherein the extension background page gets shown as combined with
the browser process, which seems wrong. I'm investigating how this might happen.

http://codereview.chromium.org/8480007/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_process_manager.h (right):

http://codereview.chromium.org/8480007/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_process_manager.h:155: Profile* profile_;
On 2011/11/06 23:13:16, Aaron Boodman wrote:
> Elsewhere in this class we get this from the browsing_context_. Is there a
need
> to store a reference to it?

No, I was just oblivious.

[Yoyo Zhou, 2011-11-08 00:49:23]

Ok, I see what's happening. The task manager gets notified about HOST_CREATED
before the extension's renderer process is created. So that's too early. Instead
I think it should get notified when ExtensionHost::RenderViewCreated is called,
and that seems to happen even if the renderer process is reused (i.e. more
correct than trying to observe a process being created).

However, I think it's still more accurate to call it HOST_CREATED than
PROCESS_CREATED. Not sure if VIEW_CREATED is better yet.

Can you take another look?

[Aaron Boodman, 2011-11-08 01:11:52]

lgtm (still)

http://codereview.chromium.org/8480007/diff/6001/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_host.cc (right):

http://codereview.chromium.org/8480007/diff/6001/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_host.cc:802:
render_view_host->process()->browser_context());
There's a profile_ pointer in ExtensionHost :-).

[Matt Perry, 2011-11-08 01:13:21]

lgtm

[Yoyo Zhou, 2011-11-08 01:40:47]

I just did more testing and I think we actually need to wait for
RenderViewReady. But I have to pack my things in order, so I'll come back to
this tomorrow.

[Yoyo Zhou, 2011-11-08 22:13:06]

On 2011/11/08 01:40:47, Yoyo Zhou wrote:
> I just did more testing and I think we actually need to wait for
> RenderViewReady. But I have to pack my things in order, so I'll come back to
> this tomorrow.

I've changed it to notify on RenderViewReady and it seems to work correctly. If
the trybots pass (it seems to be adequately covered by TaskManagerBrowserTest.*)
then I'll assume it still lgty and I'll commit.

[Matt Perry, 2011-11-08 22:16:50]

On 2011/11/08 22:13:06, Yoyo Zhou wrote:
> On 2011/11/08 01:40:47, Yoyo Zhou wrote:
> > I just did more testing and I think we actually need to wait for
> > RenderViewReady. But I have to pack my things in order, so I'll come back to
> > this tomorrow.
> 
> I've changed it to notify on RenderViewReady and it seems to work correctly.
If
> the trybots pass (it seems to be adequately covered by
TaskManagerBrowserTest.*)
> then I'll assume it still lgty and I'll commit.

Sounds good.