MHTMLGenerationManager handles the death of a render process.

MHTMLGenerationManager handles the death of a render process.

 When MHTML start to download mhtml, it does keep track of the callbacks
it will need to call when the jobs are done. But if the render process
dies while serializing a page, the manager will not be called, and the
callback will stays stall forever. This CL make sure that
MHTMLGenerationManager is getting notified when the renderer process
dies and send an error for all currently active jobs.

R=jcivelli@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=215918

[Jay Civelli, 2013-08-01 17:57:00]

https://codereview.chromium.org/21512003/diff/1/content/browser/download/mhtm...
File content/browser/download/mhtml_generation_manager.cc (right):

https://codereview.chromium.org/21512003/diff/1/content/browser/download/mhtm...
content/browser/download/mhtml_generation_manager.cc:90: }
Shouldn't we unregister for notifications for that renderer process?

https://codereview.chromium.org/21512003/diff/1/content/browser/download/mhtm...
File content/browser/download/mhtml_generation_manager.h (right):

https://codereview.chromium.org/21512003/diff/1/content/browser/download/mhtm...
content/browser/download/mhtml_generation_manager.h:42: // Implementation of
NotificationObserver.
Nit: could be moved to the private section.

[qsr, 2013-08-02 14:22:24]

https://codereview.chromium.org/21512003/diff/1/content/browser/download/mhtm...
File content/browser/download/mhtml_generation_manager.cc (right):

https://codereview.chromium.org/21512003/diff/1/content/browser/download/mhtm...
content/browser/download/mhtml_generation_manager.cc:90: }
On 2013/08/01 17:57:00, Jay Civelli wrote:
> Shouldn't we unregister for notifications for that renderer process?

Done.

https://codereview.chromium.org/21512003/diff/1/content/browser/download/mhtm...
File content/browser/download/mhtml_generation_manager.h (right):

https://codereview.chromium.org/21512003/diff/1/content/browser/download/mhtm...
content/browser/download/mhtml_generation_manager.h:42: // Implementation of
NotificationObserver.
On 2013/08/01 17:57:00, Jay Civelli wrote:
> Nit: could be moved to the private section.

Done.

[qsr, 2013-08-05 08:56:58]

gentle ping?

[Jay Civelli, 2013-08-05 17:38:50]

lgtm

[commit-bot: I haz the power, 2013-08-06 09:28:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qsr@chromium.org/21512003/7001

[commit-bot: I haz the power, 2013-08-06 09:40:25]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[qsr, 2013-08-06 09:45:11]

+ahendrickson@chromium.org for OWNERS.

[benjhayden, 2013-08-06 13:47:31]

LGTM

ahendrickson has moved on to bigger and better things, and is still in OWNERS
because he still may contribute.

[benjhayden, 2013-08-06 13:50:19]

How feasible would it be to convert this to an Observer as per the recent
announcement by phajdan.jr@chromium.org about the deprecation of
NotificationService uses in content?

If it would require some infrastructure work, then we can let them convert this
when they convert everything else in content to Observers/Delegates, so still
LGTM.

[qsr, 2013-08-06 13:58:08]

On 2013/08/06 13:50:19, benjhayden_chromium wrote:
> How feasible would it be to convert this to an Observer as per the recent
> announcement by mailto:phajdan.jr@chromium.org about the deprecation of
> NotificationService uses in content?
> 
> If it would require some infrastructure work, then we can let them convert
this
> when they convert everything else in content to Observers/Delegates, so still
> LGTM.

 I do not see any delegate or observer on the RenderProcessHost, so I do not
know how to get this information any other way.

[commit-bot: I haz the power, 2013-08-06 13:58:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qsr@chromium.org/21512003/7001

[commit-bot: I haz the power, 2013-08-06 14:21:02]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-08-06 14:26:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qsr@chromium.org/21512003/38002

[commit-bot: I haz the power, 2013-08-06 17:24:36]

Change committed as 215918