Assigning to treib@ for investigation/triage.  treib@, did something change in chrome M50 that would cause this to be the case?

lazyboy@, re if chrome disables several extensions like this, IIRC, we only have one popup, but add an error for each extension (and, as the user goes through them, another will show) - is that correct?

@1 I think so, but there was a related bug where one would replace another.

But each of them will persist in hotdog menu until any action is taken. I don't think any of them will simply get lost.

From looking at the logs, the only thing that looks possibly related is https://codereview.chromium.org/1665563003 - adding a permission message for "notifications" (which AdBlock Plus has). But that shouldn't result in a prompt if the permission was there before.

jarkko.vp.viinamaki, could you post the permission messages you get when installing the extension in Chrome 49 (step 1), and when updating to Chrome 50 (step 3)?

@treib@chromium.org

The change you referenced (adding a message for notifications) is definitely causing users' extensions to be disabled — even when the permission was there before.  We’re not seeing it impact all users, but instead it impacts all of the users who had previously upgraded to a version of our extension that required notifications permissions.

Here’s what’s happening:

- Users who installed PriceBlink prior to Nov 2015 would have installed version 4.4 or earlier, which didn’t require “notifications” permissions.

- Version 4.5 was deployed in Nov 2015, and users were updated without incident, or permission warnings.  Now their extension had “notifications” permissions. When those users got updated to Chrome 50.x, our extension was disabled without warning or notification to the user (other than the hamburger menu). So, simply upgrading to Chrome 50.x caused users, who already had extensions that required notifications, to have the extension disabled.

- We don’t believe this bug is impacting users who installed our extension after Nov 2015 (version 4.5 or later).  Presumably the difference is that the original installation took place with a build that required notifications.

Note — yesterday we deployed a version that removes the notifications permission, in hopes that we can prevent additional users from being impacted by this issue.

As you can imagine, this is having a significant impact on our business — the majority of our users are users who installed prior to Nov 2015, and thus are impacted by this bug.  Hopefully this isn’t the expected/desired behavior, and hopefully there’s a fix that can put these people back in an enabled state.

treib@, can you look into this?  If Chrome is auto-disabling extensions for a permission they already had, that's a big problem.

I'm looking.

I've verified that the new "notifications" message by itself does NOT generally cause extensions to get disabled. (In particular, what I did: In a local Chrome build, remove the new message for "notifications", install AdBlock Plus, close Chrome, add the message again, restart Chrome, see that Adblock Plus is still enabled and now shows the new message in the "Details" dialog on about:extensions.)

Re comment #5: One sequence of events that would legitimately cause the extension to get disabled is this:
- Install v4.4 (without "notifications").
- Update to Chrome 50.
- Extension gets updated to v4.5 (with "notifications"). Since the extension has a new permission with a message, it gets disabled.

I'll continue looking; it's possible there was some other change between 49 and 50 that might have caused this. I think AdBlock has had the "notifications" permission for a while (is there a way to check?), so the above sequence of events wouldn't have been the cause there.

I re-tested with actual Chrome 49 and 50 builds (versions 49.0.2578.0 and 50.0.2661.99 respectively) and still can't reproduce.

One caveat: I've tested on Linux so far. I'll try Windows now.

Checked on Windows as well, still can't reproduce.

Is it possible that AdBlock also added the notifications permission recently(ish), and ran into the situation described in comment #8? (i.e. users getting the update with the new permission after updating to Chrome 50)

Back over to Devlin, since I'm out of ideas - as far as I can tell, Chrome seems to be behaving correctly.
Is there any way to tell when exactly AdBlock added the notifications permission?

@treib The sequence we are experiencing is actually different than what you outlined in comment #8. Here is the sequence we're referring to:

- User is on Chrome 49
- User installs PriceBlink 4.4 (without notifications in manifest)
- User gets updated to PriceBlink 4.5 (with notifications in manifest). At this point they are updated with no permissions warnings. Still on Chrome 49 which obviously didn't require a warning for notifications.
- Users gets updated to Chrome 50 which now requires a warning for notifications
- PriceBlink is automatically disabled by Chrome

In your opinion is this the desired behavior? We certainly don't feel like it should be.

Thanks.

No, that is not the desired behavior. However, as detailed above I've been unable to reproduce this, so I was speculating on a different sequence of events that would explain the behavior you're seeing. It's quite possible that there's something I've missed, but the bug is not as simple as "anything with notifications permission gets disabled by Chrome 50".

Are you sure the sequence you're describing is what actually happens? How do you test/reproduce this?

Ah, forget what I said, I've reproduced it now. *Updating* the extension with the new permission before the Chrome 49->50 update is the crucial step. Repro steps:
- In Chrome 49, install an extension without "notifications".
- Update the extension to a version with "notifications".
- Update to Chrome 50, see "new permissions" prompt.

(In case anybody else tries to repro: This works with local crx files, no need to upload anything to the Web Store.)

Seems that the "notifications" permission fails to get stored during the extension update. It does get stored when it's already present during the initial install though, so that's clearly a bug.
Looking again.

Found the problem: During an extension update, no permissions are granted.
If there are new permissions that are considered a privilege increase (i.e. something that triggers a new message), then the extension is disabled (the new permissions are granted on re-enabling). However, if none of the new permissions triggers a new message, then the new permissions are in fact never granted.

Interesting that this has even worked so far... I guess the place where the permission is used doesn't actually check if it has been granted?

Anyway. Should be simple to fix for future versions, but I don't see a good way to get users out of the bad state.
I'll send a CL with the fix, but if we want to do the cleanup for users in the bad state, someone else will have to handle that.

@treib We sincerely hope you or one of your colleagues can come up with a fix that will revert impacted extensions to an enabled state.  With over 55% of our users being impacted by this bug, it’s hard to overstate the impact this is having on the business that we’ve spent the past six years building. Not to mention other extension developers that are impacted by this bug.

Incidentally, as soon as we realized what was happening, we pushed out a new update (PriceBlink 4.7) that removed the Notifications permission, hoping it would stem the flow of deactivations.  Unfortunately, even after the extension is updated, the warning (and disabled state) persist.

Thank you.

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

commit 2c20f944e2e1d615eb57d0520e25308d82d0516d
Author: treib <treib@chromium.org>
Date: Wed May 18 19:50:28 2016

Extensions: Grant nuisance and already-implied permissions on update

BUG= 603822 

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

[modify] https://crrev.com/2c20f944e2e1d615eb57d0520e25308d82d0516d/chrome/browser/extensions/extension_service.cc

Today my Chromium updated to Version 50.0.2661.102 Ubuntu 14.04 (64-bit)
 and I saw this issue at AdBlock and ScriptSafe. 

ScriptSafe [1] wasn't updated since 2014-05-19 and the "notification" permission was added to ScriptSafe at 2012-12-29 [2].

stat oiigbmnaadbkfbmpbfijlflahbdbdgdf at my extension folder says:
Access: 2016-05-20 08:57:54.522314286 +0200
Modify: 2014-05-20 09:12:55.730463570 +0200
Change: 2014-05-20 09:12:55.730463570 +0200

So I do have this extension installed for while (and at least since the last update).

@ #15 I'm not sure, does your finding explain why this is happening now and not at other Chromium updates since 2014?

Thanks.

[1] https://chrome.google.com/webstore/detail/scriptsafe/oiigbmnaadbkfbmpbfijlflahbdbdgdf
[2] https://github.com/thoroc/scriptno/commit/8810df55fa135cee1560bcd35820c68af6c9969c#diff-4b1eb3dc48c4e16d49db5b42298fe654R25 (exported from Google Code)

Re #18: If you had already ScriptSafe installed before it added the "notifications" permission, then yes, this explains it.
This happened now because Chrome 50 added a message for the "notifications" permission.

Hmm, that's hard to say, but since the oldest file at my profile is from 2011 it's reasonable to assume that a notification-less version was installed.

One (maybe) last question: will your fix work only for extension updates that are  made while the patch is present or will it fix/"cure" also earlier permission  (entry) additions that might get a warning in the future?

Thanks.

The patch will grant any missing permissions during Chrome startup, so in that sense it will "cure" the problem. However, it won't re-enable any extensions that were already disabled due to the bug.

In comment #15 treib mentions getting users out of the disabled state due to this bug. Our data shows that approximately 55% of our extension users are impacted. We had to do a quick patch of our extension that removed the notifications permission to prevent further user churn. Aside from having to roll out an update with less functionality, this bug has had a significant impact on our business. As we're seeing from this thread PriceBlink is not the only extension affected. Are there plans to re-enable these users or have we lost them for good? We understand that users can manually go in and re-enable an extension, but from our experience over the past 6 years of development, we know that many users won't do that. If there's anything that we can do from this end please let us know. Thanks.

[Automated comment] Less than 2 weeks to go before stable on M51, manual review required.

Before we approve merge to M51, Could you please confirm whether this change is baked/verified in Canary and safe to merge?

Yes, this has been in several Canary releases, and nothing major has exploded.

I think this is safe to merge, but I'd like to hear Devlin's opinion as well.
So, Devlin: WDYT?

Thank you for confirmation treib@.

If it is a safe merge and it needs to be included in M51 stable cut today, then we need to merge asap. Devlin, could you please confirm?

I think this should be merged.

Approving merge to M51 branch 2704 based on comment #27 and #30. Please merge ASAP. Thank you.

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

commit c7246fbae2d2286031f313d00f06ae2af5ff7d30
Author: Marc Treib <treib@chromium.org>
Date: Tue May 24 17:45:15 2016

Extensions: Grant nuisance and already-implied permissions on update

BUG= 603822 

Review-Url: https://codereview.chromium.org/1995533002
Cr-Commit-Position: refs/heads/master@{#394511}
(cherry picked from commit 2c20f944e2e1d615eb57d0520e25308d82d0516d)

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

Cr-Commit-Position: refs/branch-heads/2704@{#652}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/c7246fbae2d2286031f313d00f06ae2af5ff7d30/chrome/browser/extensions/extension_service.cc

Verified the fix on Windows 7, MAC (10.11.4) & Ubuntu Trusty (14.04) for Google Chrome Beta Version - 51.0.2704.63

Screen-recording is attached.

TE-Verified labels are added.

Deleted: 603822.zip 28.0 MB

603822.zip 28.0 MB Download

Procedure followed: (In MAC System case)

Steps:

1. Installed the M49 build (In my case - 49.0.2623.75 - Beta)
2. Installed the Extension - Ad Block Plus.
3. Quit Chrome (Do not uninstall)
4. Installed the M51 build (In this case - 51.0.2704.63 - Beta)
5. Replaced Chrome. Since old chrome version M49 was still installed on my system.
6. Launch the Chrome (now the Chrome Version is M51 - 51.0.2704.63)
7. No unwanted pop-up for Ad Blocked Plus extension.

Thank you.

I consider this bug fixed, since the actual problem has been resolved. I'll keep it open until a test for the new behavior has landed.

I've filed  bug 616474  for the cleanup (i.e. re-enabling extensions that have all permissions); please follow along there if you're interested.

Oops.

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

commit fef128daf132927b488fea58466ab834ccf7e1b7
Author: treib <treib@chromium.org>
Date: Thu Jun 02 09:14:51 2016

Extensions: Add test for granting nuisance and implied permissions on update

This is the test that should have accompanied https://codereview.chromium.org/1995533002

BUG= 603822 

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

[modify] https://crrev.com/fef128daf132927b488fea58466ab834ccf7e1b7/chrome/browser/extensions/extension_service_unittest.cc
[add] https://crrev.com/fef128daf132927b488fea58466ab834ccf7e1b7/chrome/test/data/extensions/permissions/update.pem
[add] https://crrev.com/fef128daf132927b488fea58466ab834ccf7e1b7/chrome/test/data/extensions/permissions/update_1/manifest.json
[add] https://crrev.com/fef128daf132927b488fea58466ab834ccf7e1b7/chrome/test/data/extensions/permissions/update_2/manifest.json
[add] https://crrev.com/fef128daf132927b488fea58466ab834ccf7e1b7/chrome/test/data/extensions/permissions/update_3/manifest.json
[add] https://crrev.com/fef128daf132927b488fea58466ab834ccf7e1b7/chrome/test/data/extensions/permissions/update_4/manifest.json

Test has landed, closing this one for realz now.

As mentioned above, if you're interested in the cleanup (re-enabling extensions that were hit by this), please follow along on  bug 616474 .

For any thread subscribers, I'm curious if this bug relates to another active bug I just filed, which looks like Chrome 51 having improper treatment of previous search_provider-using extension installs: see  bug 619759 

https://bugs.chromium.org/p/chromium/issues/detail?id=619759

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

commit 59fde879e0214d423c585ee6bc7d2edf30070bfe
Author: Marc Treib <treib@chromium.org>
Date: Mon Jun 20 13:01:05 2016

Extensions: Add test for granting nuisance and implied permissions on update

This is the test that should have accompanied https://codereview.chromium.org/1995533002

BUG= 603822 , 616474 

Review-Url: https://codereview.chromium.org/2025303002
Cr-Commit-Position: refs/heads/master@{#397344}
(cherry picked from commit fef128daf132927b488fea58466ab834ccf7e1b7)

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

Cr-Commit-Position: refs/branch-heads/2743@{#394}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/59fde879e0214d423c585ee6bc7d2edf30070bfe/chrome/browser/extensions/extension_service_unittest.cc
[add] https://crrev.com/59fde879e0214d423c585ee6bc7d2edf30070bfe/chrome/test/data/extensions/permissions/update.pem
[add] https://crrev.com/59fde879e0214d423c585ee6bc7d2edf30070bfe/chrome/test/data/extensions/permissions/update_1/manifest.json
[add] https://crrev.com/59fde879e0214d423c585ee6bc7d2edf30070bfe/chrome/test/data/extensions/permissions/update_2/manifest.json
[add] https://crrev.com/59fde879e0214d423c585ee6bc7d2edf30070bfe/chrome/test/data/extensions/permissions/update_3/manifest.json
[add] https://crrev.com/59fde879e0214d423c585ee6bc7d2edf30070bfe/chrome/test/data/extensions/permissions/update_4/manifest.json

Removing the "merge-merged-2743" label, since it's misleading - only the test was merged (to allow merging another dependent CL).

Verified the fix on Windows 7, MAC (10.11.4) & Ubuntu Trusty (14.04) for Google Chrome Beta Version - 52.0.2743.49

Screen-recording is attached.

TE-Verified labels are added.

Deleted: Screen Shot 2016-06-22 at 12.23.41 PM.png 210 KB

	Screen Shot 2016-06-22 at 12.23.41 PM.png 210 KB View Download