New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 658006 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Audit unused extension system event notification code.

Project Member Reported by msw@chromium.org, Oct 20 2016

Issue description

Audit unused extension system event notification code.

ExtensionSystemEventObserver is unused; ditto for functions it calls:
extensions::DispatchBrightnessChangedEvent
extensions::DispatchWokeUpEvent
extensions::DispatchScreenUnlockedEvent

It was added here: https://codereview.chromium.org/26692002
This CL disabled the only use, afaict: https://codereview.chromium.org/608283003
It seems like this code was disabled without being completely removed/audited.

We should audit extensions referencing this functionality, and remove whatever's unneeded.

This came up in https://codereview.chromium.org/2427913003
(that ports volume notifications to a similar class)
 

Comment 1 by st...@chromium.org, Mar 3 2017

Cc: r...@chromium.org

Comment 2 by st...@chromium.org, Mar 3 2017

Cc: -st...@chromium.org
Project Member

Comment 3 by sheriffbot@chromium.org, Mar 9 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: -tbarzic@chromium.org
Owner: tbarzic@chromium.org
Status: Assigned (was: Untriaged)
Can we just remove the class ExtensionSystemEventObserver
I looked up usage for
SYSTEM_PRIVATE_ON_BRIGHTNESS_CHANGED,
SYSTEM_PRIVATE_ON_SCREEN_UNLOCKED,
SYSTEM_PRIVATE_ON_VOLUME_CHANGED,
SYSTEM_PRIVATE_ON_WOKE_UP,
events, and didn't find any usage, so all of these can be removed. ref: http://shortn/_sdp3mUHhzj

Assigning to tbarzic@, can you or someone else take a look at this?
Cc: tengs@chromium.org
tengs - it seems that easy_unlock app uses systemPrivate.onWokeUp; well, tries to use. Chrome hasn't been dispatching the event for a while (around 3.5 years).

Given that this hasn't been causing issues, I think it should be safe to remove the event all together.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 16 2018

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

commit 82b10c1825e41707fbe93f2984678b534b211022
Author: Toni Barzic <tbarzic@chromium.org>
Date: Mon Apr 16 19:44:02 2018

Remove unused systemPrivate API events

Removes the following events:
systemPrivate.onVolumeChanged
systemPrivate.onBrightnessChanged
systemPrivate.onScreenUnlocked
systemPrivate.onWokeUp

None of them seem to be used. Additionally, code that dispatches all
listed events except systemPrivate.onVolumeChanged has been unreachable
for some time.

Bug:658006

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I104873a45b3274a7105215dd906881065a962527
Reviewed-on: https://chromium-review.googlesource.com/967421
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551079}
[modify] https://crrev.com/82b10c1825e41707fbe93f2984678b534b211022/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/82b10c1825e41707fbe93f2984678b534b211022/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/82b10c1825e41707fbe93f2984678b534b211022/chrome/browser/chromeos/chrome_browser_main_chromeos.h
[delete] https://crrev.com/a9d29a7aecf3042132247b323f79703605a272c0/chrome/browser/chromeos/extensions/extension_system_event_observer.cc
[delete] https://crrev.com/a9d29a7aecf3042132247b323f79703605a272c0/chrome/browser/chromeos/extensions/extension_system_event_observer.h
[delete] https://crrev.com/a9d29a7aecf3042132247b323f79703605a272c0/chrome/browser/chromeos/extensions/extension_volume_observer.cc
[delete] https://crrev.com/a9d29a7aecf3042132247b323f79703605a272c0/chrome/browser/chromeos/extensions/extension_volume_observer.h
[modify] https://crrev.com/82b10c1825e41707fbe93f2984678b534b211022/chrome/browser/extensions/api/system_private/system_private_api.cc
[modify] https://crrev.com/82b10c1825e41707fbe93f2984678b534b211022/chrome/browser/extensions/api/system_private/system_private_api.h
[modify] https://crrev.com/82b10c1825e41707fbe93f2984678b534b211022/chrome/common/extensions/api/system_private.json
[modify] https://crrev.com/82b10c1825e41707fbe93f2984678b534b211022/third_party/closure_compiler/externs/system_private.js

Status: Fixed (was: Assigned)
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/82b10c1825e41707fbe93f2984678b534b211022

commit 82b10c1825e41707fbe93f2984678b534b211022
Author: Toni Barzic <tbarzic@chromium.org>
Date: Mon Apr 16 19:44:02 2018

Remove unused systemPrivate API events

Removes the following events:
systemPrivate.onVolumeChanged
systemPrivate.onBrightnessChanged
systemPrivate.onScreenUnlocked
systemPrivate.onWokeUp

None of them seem to be used. Additionally, code that dispatches all
listed events except systemPrivate.onVolumeChanged has been unreachable
for some time.

Bug:658006

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I104873a45b3274a7105215dd906881065a962527
Reviewed-on: https://chromium-review.googlesource.com/967421
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551079}
[modify] https://crrev.com/82b10c1825e41707fbe93f2984678b534b211022/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/82b10c1825e41707fbe93f2984678b534b211022/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/82b10c1825e41707fbe93f2984678b534b211022/chrome/browser/chromeos/chrome_browser_main_chromeos.h
[delete] https://crrev.com/a9d29a7aecf3042132247b323f79703605a272c0/chrome/browser/chromeos/extensions/extension_system_event_observer.cc
[delete] https://crrev.com/a9d29a7aecf3042132247b323f79703605a272c0/chrome/browser/chromeos/extensions/extension_system_event_observer.h
[delete] https://crrev.com/a9d29a7aecf3042132247b323f79703605a272c0/chrome/browser/chromeos/extensions/extension_volume_observer.cc
[delete] https://crrev.com/a9d29a7aecf3042132247b323f79703605a272c0/chrome/browser/chromeos/extensions/extension_volume_observer.h
[modify] https://crrev.com/82b10c1825e41707fbe93f2984678b534b211022/chrome/browser/extensions/api/system_private/system_private_api.cc
[modify] https://crrev.com/82b10c1825e41707fbe93f2984678b534b211022/chrome/browser/extensions/api/system_private/system_private_api.h
[modify] https://crrev.com/82b10c1825e41707fbe93f2984678b534b211022/chrome/common/extensions/api/system_private.json
[modify] https://crrev.com/82b10c1825e41707fbe93f2984678b534b211022/third_party/closure_compiler/externs/system_private.js

Sign in to add a comment