Provide observer functions for NOTIFICATION_WEB_CONTENTS_CONNECTED, NOTIFICATION_WEB_CONTENTS_SWAPP…

Provide observer functions for NOTIFICATION_WEB_CONTENTS_CONNECTED, NOTIFICATION_WEB_CONTENTS_SWAPPED, NOTIFICATION_WEB_CONTENTS_DISCONNECTED

BUG=170921
TEST=no functional change

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

[Avi (use Gerrit), 2013-09-04 20:50:06]

John, can you take a look at this?

I'm looking at the existing overlap of notifications with observer functions,
but there's some weird overlap. My commentary in the notification types file is
what I'm thinking but I'm still a bit unsure WRT the connected/disconnected bit
vs notify_disconnection_.

[jam, 2013-09-04 22:34:27]

lgtm

On 2013/09/04 20:50:06, Avi wrote:
> John, can you take a look at this?
> 
> I'm looking at the existing overlap of notifications with observer functions,
> but there's some weird overlap. My commentary in the notification types file
is
> what I'm thinking but I'm still a bit unsure WRT the connected/disconnected
bit
> vs notify_disconnection_.

I haven't looked closely at this, do you want me to? I trust your reading of the
code paths. if you want me to take a closer look, let me know.

[Avi (use Gerrit), 2013-09-04 23:24:57]

On 2013/09/04 22:34:27, jam wrote:
> lgtm
> 
> On 2013/09/04 20:50:06, Avi wrote:
> > John, can you take a look at this?
> > 
> > I'm looking at the existing overlap of notifications with observer
functions,
> > but there's some weird overlap. My commentary in the notification types file
> is
> > what I'm thinking but I'm still a bit unsure WRT the connected/disconnected
> bit
> > vs notify_disconnection_.
> 
> I haven't looked closely at this, do you want me to? I trust your reading of
the
> code paths. if you want me to take a closer look, let me know.

I trust my analysis of the code paths, but I want you to look closer at the
semantics. Particularly NOTIFICATION_WEB_CONTENTS_DISCONNECTED ->
RenderProcessGone + WebContentsDestroyed. Is it safe to split it, or would it be
better to retain the redundancy but have a clear pair of connected/disconnected?

[jam, 2013-09-05 22:50:39]

On 2013/09/04 23:24:57, Avi wrote:
> On 2013/09/04 22:34:27, jam wrote:
> > lgtm
> > 
> > On 2013/09/04 20:50:06, Avi wrote:
> > > John, can you take a look at this?
> > > 
> > > I'm looking at the existing overlap of notifications with observer
> functions,
> > > but there's some weird overlap. My commentary in the notification types
file
> > is
> > > what I'm thinking but I'm still a bit unsure WRT the
connected/disconnected
> > bit
> > > vs notify_disconnection_.
> > 
> > I haven't looked closely at this, do you want me to? I trust your reading of
> the
> > code paths. if you want me to take a closer look, let me know.
> 
> I trust my analysis of the code paths, but I want you to look closer at the
> semantics. Particularly NOTIFICATION_WEB_CONTENTS_DISCONNECTED ->
> RenderProcessGone + WebContentsDestroyed. Is it safe to split it, or would it
be
> better to retain the redundancy but have a clear pair of
connected/disconnected?

I think for each listener NOTIFICATION_WEB_CONTENTS_DISCONNECTED we'd have to go
through one by one and try to look through the history to see if they need both
observer callbacks or just one.

[Avi (use Gerrit), 2013-09-06 01:14:09]

On 2013/09/05 22:50:39, jam wrote:
> On 2013/09/04 23:24:57, Avi wrote:
> > On 2013/09/04 22:34:27, jam wrote:
> > > lgtm
> > > 
> > > On 2013/09/04 20:50:06, Avi wrote:
> > > > John, can you take a look at this?
> > > > 
> > > > I'm looking at the existing overlap of notifications with observer
> > functions,
> > > > but there's some weird overlap. My commentary in the notification types
> file
> > > is
> > > > what I'm thinking but I'm still a bit unsure WRT the
> connected/disconnected
> > > bit
> > > > vs notify_disconnection_.
> > > 
> > > I haven't looked closely at this, do you want me to? I trust your reading
of
> > the
> > > code paths. if you want me to take a closer look, let me know.
> > 
> > I trust my analysis of the code paths, but I want you to look closer at the
> > semantics. Particularly NOTIFICATION_WEB_CONTENTS_DISCONNECTED ->
> > RenderProcessGone + WebContentsDestroyed. Is it safe to split it, or would
it
> be
> > better to retain the redundancy but have a clear pair of
> connected/disconnected?
> 
> I think for each listener NOTIFICATION_WEB_CONTENTS_DISCONNECTED we'd have to
go
> through one by one and try to look through the history to see if they need
both
> observer callbacks or just one.

OK, I guess since we already have to go through the users of the notification
one-by-one anyway we might as well update them.

[commit-bot: I haz the power, 2013-09-06 01:14:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/23784005/4001

[commit-bot: I haz the power, 2013-09-06 08:11:14]

Change committed as 221640