Does this reproduce with --isolate-extensions? I would expect yes, but it will be good to have confirmation. Should this be a launch blocking bug?

@1 Re launch-blocking, maybe.  If this prevents any event page extension from shutting down, that's pretty serious.  Theoretically, we have tests for this, but clearly they're missing something.

It does repro in --isolate-extensions.  It also affects Google Docs Offline.

Not sure if this matters, but both the Google Dictionary extension and Google Docs Offline create OOPIFs to google.com.  Do you know of an extension with an event page that doesn't create OOPIFs?  Would be useful to test.

And yes, this sounds important for launch.  Can anyone volunteer to take a look?

@3 One of my personal favorite testing extensions is Kalman's browser clock: https://chrome.google.com/webstore/detail/browser-clock/kjcdpojndfkkmblojbgaohcakjfbmhkm

It's super simple, but is useful for testing fundamentals while being fairly certain it's not gonna go crazy.  (And happens to have an event page with no iframes.)

@comment #3, It does not repro without an oop <iframe>.

I tried with an extension with an event page, once with --site-per-process and once with --isoloate-extensions.

Ok, glad it only affects the OOPIF case.  Nasko said he could take a look.

Adding some sample/repro notes. Following are two sample extensions you can use to load as unpacked:

1) for non-oopif case, here's an extension with event page:
Visit http://127.0.0.3/ you'll see a "Send message" button, clicking it will wake up the event page.

https://github.com/lazyboy/chromium/tree/master/tests/extensions/content_script_event_page


2) for oopif case, try this one. This mimics dictionary extension, adds a frame in background page. That frame is never visible unless you explicitly visit the background page.
 
You don't have to do anything, this extension never sleeps.
https://github.com/lazyboy/chromium/tree/master/tests/extensions/content_script_oopif_event_page

I've been digging a bit into this and here is what I have found so far. The root of the problem is incorrect accounting for the keepalive count for the extension. 

When a navigation is performed in a frame in the extension, ProcessManager::OnNetworkRequestStarted is called. It increments the keepalive count, since it comes from the extension. When the navigation is to a non-extension page, we now transfer the navigation to a different process and different RenderFrameHost. When the request is complete, ProcessManager::OnNetworkRequestDone is called. It, however, fails to decrement the keepalive count, because the RFH that is passed in as parameter is not one associated with the extension. 

The ProcessManager already keeps a set of all network requests in pending_network_requests_, so a potential solution is to modify this to be a map from request id to ExtensionHost or some other way to look up the extension. This way we aren't dependent on the RenderFrameHost to be consistent across the Start and Done event, but rather use the stable identifier - request_id.

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

commit c25500369dd9a7f49d2b7d276556b2e22ba840c8
Author: nasko <nasko@chromium.org>
Date: Thu Jun 02 07:25:53 2016

Fix event page process cleanup when running in --isolate-extensions mode.

With --isolate-extensions, navigations can start in one RenderFrameHost
and complete in another. This was not possible to happen with extensions
related RenderFrameHosts in the past, so the code wasn't able to handle
it. This CL refactors the OnNetworkRequest(Started|Stopped) to use the
navigation request id as the stable identifier instead of the associated
RenderFrameHost.

BUG= 612668 

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

[modify] https://crrev.com/c25500369dd9a7f49d2b7d276556b2e22ba840c8/chrome/browser/extensions/lazy_background_page_apitest.cc
[add] https://crrev.com/c25500369dd9a7f49d2b7d276556b2e22ba840c8/chrome/test/data/extensions/api_test/lazy_background_page/event_page_with_web_iframe/event_page.html
[add] https://crrev.com/c25500369dd9a7f49d2b7d276556b2e22ba840c8/chrome/test/data/extensions/api_test/lazy_background_page/event_page_with_web_iframe/manifest.json
[modify] https://crrev.com/c25500369dd9a7f49d2b7d276556b2e22ba840c8/extensions/browser/process_manager.cc
[modify] https://crrev.com/c25500369dd9a7f49d2b7d276556b2e22ba840c8/extensions/browser/process_manager.h

This should be fixed in tomorrow's canary, so I'm resolving it. If there are still issues, feel free to reopen or file a new one.

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

commit 3cae185174cf799005ee4d4f6147e01fad45e354
Author: nasko <nasko@chromium.org>
Date: Tue Jun 07 01:11:36 2016

Revert of Fix event page process cleanup when running in --isolate-extensions mode. (patchset #6 id:100001 of https://codereview.chromium.org/2028873002/ )

Reason for revert:
Speculative revert of this CL to check if it is the root cause of https://crbug.com/616989.

Original issue's description:
> Fix event page process cleanup when running in --isolate-extensions mode.
>
> With --isolate-extensions, navigations can start in one RenderFrameHost
> and complete in another. This was not possible to happen with extensions
> related RenderFrameHosts in the past, so the code wasn't able to handle
> it. This CL refactors the OnNetworkRequest(Started|Stopped) to use the
> navigation request id as the stable identifier instead of the associated
> RenderFrameHost.
>
> BUG= 612668 
>
> Committed: https://crrev.com/c25500369dd9a7f49d2b7d276556b2e22ba840c8
> Cr-Commit-Position: refs/heads/master@{#397328}

TBR=rdevlin.cronin@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 612668 

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

[modify] https://crrev.com/3cae185174cf799005ee4d4f6147e01fad45e354/chrome/browser/extensions/lazy_background_page_apitest.cc
[delete] https://crrev.com/5824d7906802051e8c2193b05b83cb44494379ae/chrome/test/data/extensions/api_test/lazy_background_page/event_page_with_web_iframe/event_page.html
[delete] https://crrev.com/5824d7906802051e8c2193b05b83cb44494379ae/chrome/test/data/extensions/api_test/lazy_background_page/event_page_with_web_iframe/manifest.json
[modify] https://crrev.com/3cae185174cf799005ee4d4f6147e01fad45e354/extensions/browser/process_manager.cc
[modify] https://crrev.com/3cae185174cf799005ee4d4f6147e01fad45e354/extensions/browser/process_manager.h

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

commit 211f46e6aa65ae29bec721c32e8aadbf2d625e9c
Author: Scott Graham <scottmg@chromium.org>
Date: Tue Jun 07 05:13:02 2016

Revert of Fix event page process cleanup when running in --isolate-extensions mode. (patchset #6 id:100001 of https://codereview.chromium.org/2028873002/ )

Reason for revert:
Speculative revert of this CL to check if it is the root cause of https://crbug.com/616989.

Original issue's description:
> Fix event page process cleanup when running in --isolate-extensions mode.
>
> With --isolate-extensions, navigations can start in one RenderFrameHost
> and complete in another. This was not possible to happen with extensions
> related RenderFrameHosts in the past, so the code wasn't able to handle
> it. This CL refactors the OnNetworkRequest(Started|Stopped) to use the
> navigation request id as the stable identifier instead of the associated
> RenderFrameHost.
>
> BUG= 612668 
>
> Committed: https://crrev.com/c25500369dd9a7f49d2b7d276556b2e22ba840c8
> Cr-Commit-Position: refs/heads/master@{#397328}

TBR=rdevlin.cronin@chromium.org
BUG= 612668 

Review-Url: https://codereview.chromium.org/2044643003
Cr-Commit-Position: refs/heads/master@{#398185}
(cherry picked from commit 3cae185174cf799005ee4d4f6147e01fad45e354)

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

Cr-Commit-Position: refs/branch-heads/2761@{#2}
Cr-Branched-From: afd97ff8f20acb90a6b495776e385411371651f0-refs/heads/master@{#398174}

[modify] https://crrev.com/211f46e6aa65ae29bec721c32e8aadbf2d625e9c/chrome/browser/extensions/lazy_background_page_apitest.cc
[delete] https://crrev.com/d81daaa9263fbcd77aa82653fa469d8c7dcb80d6/chrome/test/data/extensions/api_test/lazy_background_page/event_page_with_web_iframe/event_page.html
[delete] https://crrev.com/d81daaa9263fbcd77aa82653fa469d8c7dcb80d6/chrome/test/data/extensions/api_test/lazy_background_page/event_page_with_web_iframe/manifest.json
[modify] https://crrev.com/211f46e6aa65ae29bec721c32e8aadbf2d625e9c/extensions/browser/process_manager.cc
[modify] https://crrev.com/211f46e6aa65ae29bec721c32e8aadbf2d625e9c/extensions/browser/process_manager.h

Marking started since it has been reverted, but it looks like you'll probably be able to reland it since https://crbug.com/616989 is apparently still not fixed in 53.0.2761.2 (which contained scottmg's merge of the revert).

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

commit d13fd29dbd8d0c7f09e527e704756eceada26fa0
Author: nasko <nasko@chromium.org>
Date: Tue Jun 07 20:33:33 2016

Reland of Fix event page process cleanup when running in --isolate-extensions mode. (patchset #1 id:1 of https://codereview.chromium.org/2044643003/ )

Reason for revert:
Reverting the revert, as the speculation of this CL being the root cause did not pan out - https://bugs.chromium.org/p/chromium/issues/detail?id=616989#c36.

Original issue's description:
> Revert of Fix event page process cleanup when running in --isolate-extensions mode. (patchset #6 id:100001 of https://codereview.chromium.org/2028873002/ )
>
> Reason for revert:
> Speculative revert of this CL to check if it is the root cause of https://crbug.com/616989.
>
> Original issue's description:
> > Fix event page process cleanup when running in --isolate-extensions mode.
> >
> > With --isolate-extensions, navigations can start in one RenderFrameHost
> > and complete in another. This was not possible to happen with extensions
> > related RenderFrameHosts in the past, so the code wasn't able to handle
> > it. This CL refactors the OnNetworkRequest(Started|Stopped) to use the
> > navigation request id as the stable identifier instead of the associated
> > RenderFrameHost.
> >
> > BUG= 612668 
> >
> > Committed: https://crrev.com/c25500369dd9a7f49d2b7d276556b2e22ba840c8
> > Cr-Commit-Position: refs/heads/master@{#397328}
>
> TBR=rdevlin.cronin@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG= 612668 
>
> Committed: https://crrev.com/3cae185174cf799005ee4d4f6147e01fad45e354
> Cr-Commit-Position: refs/heads/master@{#398185}

TBR=rdevlin.cronin@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 612668 

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

[modify] https://crrev.com/d13fd29dbd8d0c7f09e527e704756eceada26fa0/chrome/browser/extensions/lazy_background_page_apitest.cc
[add] https://crrev.com/d13fd29dbd8d0c7f09e527e704756eceada26fa0/chrome/test/data/extensions/api_test/lazy_background_page/event_page_with_web_iframe/event_page.html
[add] https://crrev.com/d13fd29dbd8d0c7f09e527e704756eceada26fa0/chrome/test/data/extensions/api_test/lazy_background_page/event_page_with_web_iframe/manifest.json
[modify] https://crrev.com/d13fd29dbd8d0c7f09e527e704756eceada26fa0/extensions/browser/process_manager.cc
[modify] https://crrev.com/d13fd29dbd8d0c7f09e527e704756eceada26fa0/extensions/browser/process_manager.h

Resolving as fixed again, since the CL has been relanded.