-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: EME Logger Manifest V3 Migration #51
Merged
joeyparrish
merged 4 commits into
shaka-project:main
from
beback4u:manifest_v3_migration
Aug 1, 2022
Merged
feat: EME Logger Manifest V3 Migration #51
joeyparrish
merged 4 commits into
shaka-project:main
from
beback4u:manifest_v3_migration
Aug 1, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
beback4u
requested review from
joeyparrish,
jrummell-chromium and
xhwang-chromium
July 13, 2022 21:18
beback4u
force-pushed
the
manifest_v3_migration
branch
2 times, most recently
from
July 28, 2022 07:45
9ca26f5
to
0c2c704
Compare
beback4u
changed the title
chore: EME Logger Manifest V3 Migration
feat: EME Logger Manifest V3 Migration
Jul 28, 2022
joeyparrish
requested changes
Jul 28, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great overall! Thanks for doing this!
joeyparrish
requested changes
Jul 29, 2022
joeyparrish
approved these changes
Aug 1, 2022
beback4u
pushed a commit
that referenced
this pull request
Mar 30, 2023
🤖 I have created a release *beep* *boop* --- ## [3.3.0](v3.2.0...v3.3.0) (2023-01-10) ### Features * EME Logger Manifest V3 Migration ([#51](#51)) ([c860eba](c860eba)) ### Bug Fixes * Display keystatuseschange.expiration as a Date ([#48](#48)) ([d5d15f6](d5d15f6)), closes [#42](#42) * Remove all variables and most functions from global scope ([#61](#61)) ([3b8ed85](3b8ed85)), closes [#44](#44) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions
bot
added
the
status: archived
Archived and locked; will not be updated
label
Jul 25, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
EME Logger Manifest V3 Migration
Summary
The Chrome browser will no longer run Manifest V2 extensions after January 2023 as per The transition of Chrome extensions to Manifest V3. Hence, EME Logger's Manifest version needs to be updated from V2 to V3:
chrome.extension.getURL
is replaced withchrome.runtime.getURL
.window.EmeLogWindow = EmeLogWindow;
becausewindow
is not defined in the log_window.js since the script is running in a service worker (a separate thread). Now the singleton instance of EmeLogWindow (EmeLogWindow.instance
) lives in the service worker.window.open
withchrome.windows.create
to create a new windowchrome.browerAction
is replaced withchrome.action
.document
or DOM object. As a result,EmeLogWindow.appendLog()
andclear()
code dealing with the document object has been moved to the log window script (log.js)chrome.downloads.download
APIwith adding "downloads" to the "permission" section in the manifest file sinceURL.createObjectURL
is not available in a service worker.web_accessible_resources
is now an array of objects instead of a single object.browser_action
is replaced withaction
.matches
in theweb_accessible_resources
section because no log is getting collected due to the permission issue.chrome.runtime.onMessage.addListener
andchrome.runtime.sendMessage
APIs to communicate because the pop-up window (log.js) cannot access the EmeLogWindow instance in the service worker (log-window.js).chrome.windows.onRemoved.addListener
to observe the termination of window because there is no.closed
property in the window created bychrome.windows.create
.chrome.extension.getURL
is replaced withchrome.runtime.getURL
.chrome.runtime
andchrome.windows
APIs because Jasmine-browser and Selenium-webdriver are not working with chrome APIs and we need test code changes to test async calls.npm run build & npm test
complains about the out-dated versions of selenium-webdriver and other dependenciesEmeLogWindow.instance.open()
API tests. Otherwise, EmeLogWindow.instance.isOpen()'s return value is not guaranteed since.open()
is an async operation.EmeLogWindow.instance.appendLog()
andEmeLogWindow.instance.clear()
APIssrcFiles
section because we cannot directly test theappendLog()
logic without the chrome messaging APIs that are not available on the Jasmine framework. As a workaround, log-window-tests.js calls theappendLog()
method defined in the log.js first and passes it downEmeLogWindow.appendLog()
in the tests.npm test
. Other dependencies are updated together.Test Plan
All unit tests with 44 specs were passed by
npm test
. Integration tests were manually done on the MacOSX.Unit Tests
Test results:
Integration Tests
Prerequisites:
npm list
command)Test steps:
Test results:
Migration Issues
1. Error: The "background.scripts" key cannot be used with manifest_version 3. Use the "background.service_worker" key instead.
Failed to load extension: Could not load manifest.
File: ~/github/eme_logger
https://stackoverflow.com/questions/66055882/chrome-extensions-use-the-background-service-worker-key-instead-manifest-vers
https://developer.chrome.com/docs/extensions/mv3/intro/mv3-migration/#man-sw
https://developer.chrome.com/docs/extensions/mv3/migrating_to_service_workers/
--> Replace "scripts" with "service_worker" for the "background" configuration
2. Error: Error at key 'web_accessible_resources'. Parsing array failed at index 0: expected dictionary, got string
Failed to load extension: Could not load manifest.
File: ~/github/eme_logger
https://developer.chrome.com/docs/extensions/mv3/intro/mv3-migration/#web-accessible-resources
--> Provide an array of objects instead of a single object
3. 'browser_action' requires manifest version of 2 or lower.
https://developer.chrome.com/docs/extensions/mv3/intro/mv3-migration/#action-api-unification
--> Use
action
API instead4. Uncaught TypeError: chrome.extension.getURL is not a function
content-script.js:22 (anonymous function)
https://developer.chrome.com/docs/extensions/mv3/mv3-migration-checklist/#api_checklist
--> Use
chrome.runtime.getURL()
to build resource URLs at runtime.5. Service worker registration failed
Uncaught ReferenceError: window is not defined
https://stackoverflow.com/questions/68194103/error-in-event-handler-referenceerror-window-is-not-defined-chrome-extension-w
ManifestV3 extension uses a service worker so it doesn't have DOM or
window
.https://developer.chrome.com/docs/extensions/mv3/intro/mv3-migration/#background-service-workers
https://developer.chrome.com/docs/workbox/service-worker-overview/
https://developer.chrome.com/docs/extensions/mv3/migrating_to_service_workers/
https://stackoverflow.com/questions/28799892/how-to-launch-a-new-window-in-google-chrome-extension
--> Use
chrome.windows.getCurrent
to get the size/position of the current browser window (only for window info).--> Use
chrome.windows.create
to create a new window.--> Remove this line
window.EmeLogWindow = EmeLogWindow;
since the singleton instance of EmeLogWindow (EmeLogWindow.instance
) lives in the service worker.6. Uncaught TypeError: Cannot read properties of undefined (reading 'onClicked')
https://developer.chrome.com/docs/extensions/mv3/migrating_to_service_workers/#event_listeners
--> Manifest V3 consolidates chrome.browserAction and chrome.pageAction into a single
chrome.action
API.7. Cannot access an instance of EmeLogWindow from a pop-up window (log.js) to the service worker (log-window.js).
https://developer.chrome.com/docs/extensions/mv3/messaging/
https://www.freecodecamp.org/news/chrome-extension-message-passing-essentials/
--> Use
chrome.runtime.onMessage.addListener
andchrome.runtime.sendMessage
APIs instead8. Uncaught TypeError: Cannot read properties of undefined (reading 'closed')
this.logWindow_.closed
in theEmeLogWindow
class.--> Use
chrome.windows.onRemoved.addListener
to observe the termination of window9. Cannot access the
document
or DOM object from a service worker.https://stackoverflow.com/questions/4532236/how-to-access-the-webpage-dom-rather-than-the-extension-page-dom
Issue 1191971: Chrome extension: chrome.scripting.executeScript not working in my manifest v3 Chrome Extension: https://bugs.chromium.org/p/chromium/issues/detail?id=1191971
Chrome extension injecting script get error: https://stackoverflow.com/questions/36762389/chrome-extension-injecting-script-get-error
Resolved: Extension manifest must request permission to access the respective host or a different Error: https://dailydevsblog.com/troubleshoot/resolved-extension-manifest-must-request-permission-to-access-the-respective-host-or-a-different-error-53911/
https://stackoverflow.com/questions/53024819/chrome-extension-sendresponse-not-waiting-for-async-function
How to Send Data Between Chrome Extension Scripts
--> If we use
chrome.scripting.executeScript
instead, it didn't work with an error "log-window.js:76 Uncaught (in promise) Error: Cannot access contents of the page. Extension manifest must request permission to access the respective host." error--> Move the logic handling DOM from
log-window.js
(service worker) tolog.js
(log window) and communicate between two scripts viachrome.runtime.sendMessage
andchrome.runtime.onMessage.addListener
APIs10. Error in event handler: TypeError: URL.createObjectURL is not a function at EmeLogWindow.getTextLogUri
Issue 1224027: Creating a download from generated content in a ManifestV3 service worker
TypeError: Error URL.createObjectURL() not a function
https://stackoverflow.com/questions/4771758/how-to-generate-a-file-for-download-in-a-google-chrome-extension
--> The download won't get triggered because the messaging is an async operation if we use
string.replace()
simply to doreturn 'data:text/plain,' + this.textLogs_.replace("#", "%23");
--> Use
chrome.downloads.download
API with adding "downloads" to the "permission" section in the manifest file.11. No log is getting collected
--> Add "<all_urls>" into "matches" in the "web_accessible_resources" section in the manifest file.
12. Error after clicking "Download log" button: "Unchecked runtime.lastError: A listener indicated an asynchronous response by returning true, but the message channel closed before a response was received"
--> Turned out we should not do
return true;
to keep the messaging channel open in the message handler ofchrome.runtime.onMessage.addListener
13. 23 test failures when running npm test with Jasmine-browser and Selenium-webdriver
https://stackoverflow.com/questions/19394459/unable-to-access-chrome-message-passing-api-from-selenium-execute-script
https://stackoverflow.com/questions/45637489/how-to-test-a-function-which-calls-async-function-using-jasmine
https://howtodoinjava.com/javascript/jasmine-unit-testing-tutorial/
Unit Testing Async Calls and Promises with Jasmine
--> Set up the mock/fake properties/methods for
chrome.runtime
andchrome.windows
APIs: Jasmine-browser and Selenium-webdriver are not working with chrome.xxx APIs and we need test code changes to test async calls.--> Updated the required dependency packages:
npm run build & npm test
complains about the out-dated versions of selenium-webdriver and other dependencies--> Used Promise/async-wait patterns for
EmeLogWindow.instance.open()
API tests--> Added new test cases for
EmeLogWindow.instance.appendLog()
andEmeLogWindow.instance.clear()
APIs14. Invalid URL by selenium-webdriver when running npm test
--> Updated selenium-webdriver to the latest version (4.3.1)
References