Remove deprecated extension notification from Browser

Remove deprecated extension notification from Browser.

This patch used EntensionRegistryObserver instead of DEPRECATED extension notification. Now those below
DEPRECATED notifications removed.
-- NOTIFICATION_EXTENSION_LOADED_DEPRECATED
-- NOTIFICATION_EXTENSION_UNINSTALLED_DEPRECATED
-- NOTIFICATION_EXTENSION_UNLOADED_DEPRECATED.

BUG=411568
Committed: https://crrev.com/a00555386f6578cd2cf1ba746fc5a104183c2548
Cr-Commit-Position: refs/heads/master@{#298390}

[Jitu( very slow this week), 2014-09-10 05:14:21]

jitendra.ks@samsung.com changed reviewers:
+ bauerb@chromium.org, pkasting@chromium.org, thestig@chromium.org

[Jitu( very slow this week), 2014-09-10 05:14:42]

PTAL ...

[Bernhard Bauer, 2014-09-10 08:03:43]

In particular if you have multiple patches for this, please add to the subject
where you are making these changes (e.g. Browser).

https://codereview.chromium.org/562533002/diff/20001/chrome/browser/ui/browse...
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/562533002/diff/20001/chrome/browser/ui/browse...
chrome/browser/ui/browser.cc:1930: // Notification observer for Extension
implementation
This comment doesn't explain anything that can't be gathered from the header.

[Jitu( very slow this week), 2014-09-10 09:56:34]

Changed as per suggested.

PTAL ...

While running this in trybot i got below errors for linux and mac platform.

/chrome/browser/ui/browser.cc:355:7: error: field 'weak_factory_' will be
initialized after field 'extension_registry_observer_' [-Werror,-Wreorder]

Do we need to maintain any kind of order here ? 
If we need to maintain how i can decide which order.

Please let me know.

[Bernhard Bauer, 2014-09-10 10:39:09]

On 2014/09/10 09:56:34, Jitu wrote:
> Changed as per suggested.
> 
> PTAL ...
> 
> While running this in trybot i got below errors for linux and mac platform.
> 
> /chrome/browser/ui/browser.cc:355:7: error: field 'weak_factory_' will be
> initialized after field 'extension_registry_observer_' [-Werror,-Wreorder]
> 
> Do we need to maintain any kind of order here ? 
> If we need to maintain how i can decide which order.
> 
> Please let me know.

You need to initialize your fields in the same order in which they're declared
in the header. For the most part, the actual order is irrelevant (as long as
it's the same in the header and in the implementation), but specifically for
weak pointer factories, they should be the last member in the class, so they're
destroyed first (and therefore invalidate weak pointers, which avoids callbacks
into a half-destroyed object during destruction).

[Jitu( very slow this week), 2014-09-10 12:20:39]

Thanks Bauer for suggestion,

PTAL ...

[Bernhard Bauer, 2014-09-10 12:32:04]

https://codereview.chromium.org/562533002/diff/120001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.h (right):

https://codereview.chromium.org/562533002/diff/120001/chrome/browser/ui/brows...
chrome/browser/ui/browser.h:710: // Called when an extension is uninstalled:
These comments are unnecessary. Just add back the old comment that refered to
the class which is overridden. (And adapt to the style used here: "Overridden
from extensions::ExtensionRegistryObserver:")

[Jitu( very slow this week), 2014-09-10 12:37:42]

PTAL

[Bernhard Bauer, 2014-09-10 13:25:49]

On 2014/09/10 08:03:43, Bernhard Bauer wrote:
> In particular if you have multiple patches for this, please add to the subject
> where you are making these changes (e.g. Browser).

^^^

[Jitu( very slow this week), 2014-09-10 13:46:08]

On 2014/09/10 13:25:49, Bernhard Bauer wrote:
> On 2014/09/10 08:03:43, Bernhard Bauer wrote:
> > In particular if you have multiple patches for this, please add to the
subject
> > where you are making these changes (e.g. Browser).
> 
> ^^^

When i have started working on this bug
(https://code.google.com/p/chromium/issues/detail?id=411568), i thought of to
do the changes by file wise as multiple patch.
But those multiple patches are independent to each other.

After your comments i have modified the subject to "Remove deprecated extension
notification from Browser".

Is this you are asking? Sorry if i couldn't understand you clearly.

Please just elaborate this.

[Jitu( very slow this week), 2014-09-10 13:50:31]

On 2014/09/10 13:46:08, Jitu wrote:
> On 2014/09/10 13:25:49, Bernhard Bauer wrote:
> > On 2014/09/10 08:03:43, Bernhard Bauer wrote:
> > > In particular if you have multiple patches for this, please add to the
> subject
> > > where you are making these changes (e.g. Browser).
> > 
> > ^^^
> 
> When i have started working on this bug
> (https://code.google.com/p/chromium/issues/detail?id=411568), i thought of to
> do the changes by file wise as multiple patch.
> But those multiple patches are independent to each other.
> 
> After your comments i have modified the subject to "Remove deprecated
extension
> notification from Browser".
> 
> Is this you are asking? Sorry if i couldn't understand you clearly.
> 
> Please just elaborate this.

I have already changed the subject, but still i am receiving the mails with
older subject line.

[Bernhard Bauer, 2014-09-10 14:23:08]

On 2014/09/10 13:50:31, Jitu wrote:
> On 2014/09/10 13:46:08, Jitu wrote:
> > On 2014/09/10 13:25:49, Bernhard Bauer wrote:
> > > On 2014/09/10 08:03:43, Bernhard Bauer wrote:
> > > > In particular if you have multiple patches for this, please add to the
> > subject
> > > > where you are making these changes (e.g. Browser).
> > > 
> > > ^^^
> > 
> > When i have started working on this bug
> > (https://code.google.com/p/chromium/issues/detail?id=411568), i thought of
to
> > do the changes by file wise as multiple patch.
> > But those multiple patches are independent to each other.
> > 
> > After your comments i have modified the subject to "Remove deprecated
> extension
> > notification from Browser".
> > 
> > Is this you are asking? Sorry if i couldn't understand you clearly.
> > 
> > Please just elaborate this.
> 
> I have already changed the subject, but still i am receiving the mails with
> older subject line.

Oh, I see. Yeah, Rietveld doesn't change the subject anymore (which has the
advantage that it won't create new threads), maybe that threw me off.

Can you also update the description? That will eventually become the commit
message, so its first line should match the CL title.

[Peter Kasting, 2014-09-10 18:40:21]

You listed multiple reviewers for this change.  When you do so, please
explicitly say what you want each to review.  Normally we don't overlap
reviewers on the same pieces of code except for e.g. a non-OWNER reviewer plus
an OWNER (if the non-OWNER is a better reviewer for the change in general), so
if you just listed multiple people in hopes of finding someone to review,
consider pruning your list instead.

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.cc (left):

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
chrome/browser/ui/browser.cc:2002: // completed, at which point window_ is NULL.
See 94752 for details.
Nit: Please copy this comment to your new code location.

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
chrome/browser/ui/browser.cc:1905: // Browser, ui::SelectFileDialog::Listener
implementation:
Nit: I'm not a fan of these big dividers between subclass implementation
sections, but we should be consistent; either add one before your new functions,
or remove all of them in this file.  (Personally, I'd do the latter.)

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
chrome/browser/ui/browser.cc:1930: // Called when an extension is uninstalled
Nit: Don't add comments like this.  If they're important, they belong in the .h
file, and they need to be complete sentences.  In this case, though, the comment
just restates the function name, so it doesn't add anything to the code.  Just
remove these outright.  (3 places)

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.h (right):

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
chrome/browser/ui/browser.h:710: // Overridden from
extensions::ExtensionRegistryObserver:
Nit: Put these after the NotificationObserver overrides (here and in the .cc
file) because in the list of parent classes this superclass is listed after that
one.

Really, all the groups of overrides here should follow that ordering, but I
won't demand you fix that while you're touching this; you can if you want to.

[Jitu( very slow this week), 2014-09-11 06:08:16]

Hi Peter,

I have fixed all the review comments except one. Which i have not understand
clearly. can you please clarify that.

Apart from that i have maintain same order as functions declared in header.
	
Peter Kasting 2014/09/10 18:40:20
Nit: I'm not a fan of these big dividers between subclass implementation
sections, but we should be consistent; either add one before your new functions,
or remove all of them in this file. (Personally, I'd do the latter.) 

Thanks

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.cc (left):

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
chrome/browser/ui/browser.cc:2002: // completed, at which point window_ is NULL.
See 94752 for details.
On 2014/09/10 18:40:20, Peter Kasting wrote:
> Nit: Please copy this comment to your new code location.

Done.

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
chrome/browser/ui/browser.cc:1905: // Browser, ui::SelectFileDialog::Listener
implementation:
On 2014/09/10 18:40:20, Peter Kasting wrote:
> Nit: I'm not a fan of these big dividers between subclass implementation
> sections, but we should be consistent; either add one before your new
functions,
> or remove all of them in this file.  (Personally, I'd do the latter.)

Dear Peter,
I have not touched this function. So as per your comments , can you please
clarify where exactly i need to move this function.

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
chrome/browser/ui/browser.cc:1930: // Called when an extension is uninstalled
On 2014/09/10 18:40:20, Peter Kasting wrote:
> Nit: Don't add comments like this.  If they're important, they belong in the
.h
> file, and they need to be complete sentences.  In this case, though, the
comment
> just restates the function name, so it doesn't add anything to the code.  Just
> remove these outright.  (3 places)

Done.

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.h (right):

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
chrome/browser/ui/browser.h:710: // Overridden from
extensions::ExtensionRegistryObserver:
On 2014/09/10 18:40:21, Peter Kasting wrote:
> Nit: Put these after the NotificationObserver overrides (here and in the .cc
> file) because in the list of parent classes this superclass is listed after
that
> one.
> 
> Really, all the groups of overrides here should follow that ordering, but I
> won't demand you fix that while you're touching this; you can if you want to.

Done.

[Bernhard Bauer, 2014-09-11 08:09:21]

> Can you also update the description? That will eventually become the commit
> message, so its first line should match the CL title.

^^^

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/562533002/diff/140001/chrome/browser/ui/brows...
chrome/browser/ui/browser.cc:1905: // Browser, ui::SelectFileDialog::Listener
implementation:
On 2014/09/11 06:08:16, Jitu wrote:
> On 2014/09/10 18:40:20, Peter Kasting wrote:
> > Nit: I'm not a fan of these big dividers between subclass implementation
> > sections, but we should be consistent; either add one before your new
> functions,
> > or remove all of them in this file.  (Personally, I'd do the latter.)
> 
> Dear Peter,
> I have not touched this function. So as per your comments , can you please
> clarify where exactly i need to move this function.

IIUC, Peter's comment is not about the function, but about this comment that
separates different sections. To be consistent, there should be either one
before the ExtensionRegistryObserver methods as well, or none at all throughout
the whole file.

[Jitu( very slow this week), 2014-09-11 08:32:19]

Thanks Bauer

PTAL

[Bernhard Bauer, 2014-09-11 08:48:08]

LG, now you'll have to wait for Peter to approve.

[Peter Kasting, 2014-09-11 20:25:22]

LGTM

[Jitu( very slow this week), 2014-09-12 04:57:59]

The CQ bit was checked by jitendra.ks@samsung.com

[commit-bot: I haz the power, 2014-09-12 04:58:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/562533002/180001

[commit-bot: I haz the power, 2014-09-12 06:07:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-12 06:07:52]

Try jobs failed on following builders:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Jitu( very slow this week), 2014-09-12 07:07:13]

The CQ bit was checked by jitendra.ks@samsung.com

[Jitu( very slow this week), 2014-09-12 07:07:17]

The CQ bit was unchecked by jitendra.ks@samsung.com

[Jitu( very slow this week), 2014-09-12 08:27:31]

Dear Bernard/Peter,
Some test cases are flaking while landing this patch for linux and mac .

New failures:
ProfileManagerBrowserTest.EphemeralProfile
WindowOpenPanelTest.CloseNonExtensionPanelsOnUninstall.

Any solution for this ??

Do i need to disable those testcases for linux and mac.

Please suggest me.

[Bernhard Bauer, 2014-09-12 08:33:40]

On 2014/09/12 08:27:31, Jitu wrote:
> Dear Bernard/Peter,
> Some test cases are flaking while landing this patch for linux and mac .
> 
> New failures:
> ProfileManagerBrowserTest.EphemeralProfile
> WindowOpenPanelTest.CloseNonExtensionPanelsOnUninstall.
> 
> Any solution for this ??
> 
> Do i need to disable those testcases for linux and mac.
> 
> Please suggest me.

I think the test is crashing, and it's a release build, so it doesn't give a
proper stack trace :-/

I would suggest running the test locally (with a debug build) to find out what's
going on.

[Jitu( very slow this week), 2014-09-17 11:02:13]

On 2014/09/12 08:33:40, Bernhard Bauer wrote:
> On 2014/09/12 08:27:31, Jitu wrote:
> > Dear Bernard/Peter,
> > Some test cases are flaking while landing this patch for linux and mac .
> > 
> > New failures:
> > ProfileManagerBrowserTest.EphemeralProfile
> > WindowOpenPanelTest.CloseNonExtensionPanelsOnUninstall.
> > 
> > Any solution for this ??
> > 
> > Do i need to disable those testcases for linux and mac.
> > 
> > Please suggest me.
> 
> I think the test is crashing, and it's a release build, so it doesn't give a
> proper stack trace :-/
> 
> I would suggest running the test locally (with a debug build) to find out
what's
> going on.

Some problem in my linux build setup. Once resolve i will fix this.

[Jitu( very slow this week), 2014-10-06 10:17:31]

Patchset #6 (id:100001) has been deleted

[Jitu( very slow this week), 2014-10-06 14:03:12]

PTAL

[Lei Zhang, 2014-10-06 19:13:03]

The CQ bit was checked by thestig@chromium.org

[Lei Zhang, 2014-10-06 19:13:04]

lgtm

[commit-bot: I haz the power, 2014-10-06 19:13:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/562533002/200001

[commit-bot: I haz the power, 2014-10-06 22:09:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-06 22:09:36]

Try jobs failed on following builders:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)

[Jitu( very slow this week), 2014-10-07 05:48:30]

The CQ bit was checked by jitendra.ks@samsung.com

[commit-bot: I haz the power, 2014-10-07 05:49:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/562533002/200001

[commit-bot: I haz the power, 2014-10-07 05:54:52]

Committed patchset #10 (id:200001) as 0718c41494b5544884ccaaf56c21b9c0af9c9f0f

[commit-bot: I haz the power, 2014-10-07 05:56:06]

Patchset 10 (id:??) landed as
https://crrev.com/a00555386f6578cd2cf1ba746fc5a104183c2548
Cr-Commit-Position: refs/heads/master@{#298390}