Preserve hosted apps' background contents across reload

Use UnloadedExtensionInfo to keep hosted apps' background contents through a reload.

BUG=63426
TEST=manual

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

[Andrew T Wilson (Slow), 2011-01-14 01:41:34]

LGTM, pending tests.

http://codereview.chromium.org/6297002/diff/4001/chrome/browser/background_co...
File chrome/browser/background_contents_service.cc (right):

http://codereview.chromium.org/6297002/diff/4001/chrome/browser/background_co...
chrome/browser/background_contents_service.cc:125: // Leave background content
in place
nit: should be "BackgroundContents" not "background content".

[The wrong rickcam account, 2011-01-14 02:13:35]

Still pending testing.

http://codereview.chromium.org/6297002/diff/4001/chrome/browser/background_co...
File chrome/browser/background_contents_service.cc (right):

http://codereview.chromium.org/6297002/diff/4001/chrome/browser/background_co...
chrome/browser/background_contents_service.cc:125: // Leave background content
in place
On 2011/01/14 01:41:34, Andrew T Wilson wrote:
> nit: should be "BackgroundContents" not "background content".

Done.

[The wrong rickcam account, 2011-01-14 02:28:08]

Please take a look.

Antony, Drew has given me an LGTM on this, but I'd appreciate it if you could
take a quick look as well.  Thanks.

I've tested this using
http://www.corp.google.com/~asargent/extensions/autoupdate.html

[asargent_no_longer_on_chrome, 2011-01-15 00:09:53]

lgtm

http://codereview.chromium.org/6297002/diff/9001/chrome/browser/background_co...
File chrome/browser/background_contents_service.cc (right):

http://codereview.chromium.org/6297002/diff/9001/chrome/browser/background_co...
chrome/browser/background_contents_service.cc:128: NOTREACHED();
Would it make sense to shut down the background contents in the default case
here? I'm thinking of the possibility that in the future someone adds a new
reason type to UnloadedExtensionInfo and forgets to update your code here. I.e.
in release builds, where the NOTREACHED won't fire, would we rather err on the
side of the background contents getting shut down than left running? Your call -
just raising it for you to think about.

[Andrew T Wilson (Slow), 2011-01-15 00:37:43]

Yeah, that's a great question. I suspect you're right, and we'd rather have the
BC shutdown rather than left running (assuming that most UNLOADED notifications
will be related to the extension really going away).

[The wrong rickcam account, 2011-01-18 20:08:49]

I addressed the NOTREACHED case as suggested and will move forward to dcommit
this patch.

http://codereview.chromium.org/6297002/diff/9001/chrome/browser/background_co...
File chrome/browser/background_contents_service.cc (right):

http://codereview.chromium.org/6297002/diff/9001/chrome/browser/background_co...
chrome/browser/background_contents_service.cc:128: NOTREACHED();
On 2011/01/15 00:09:53, Antony Sargent wrote:
> Would it make sense to shut down the background contents in the default case
> here? I'm thinking of the possibility that in the future someone adds a new
> reason type to UnloadedExtensionInfo and forgets to update your code here.
I.e.
> in release builds, where the NOTREACHED won't fire, would we rather err on the
> side of the background contents getting shut down than left running? Your call
-
> just raising it for you to think about. 

Done.  I inserted a call to ShutdownAssociatedBackgroundContents immediately
following the NOTREACHED().