Conversation
18 tasks
7edb43d to
1fbba6e
Compare
0331f22 to
678c528
Compare
16 tasks
Member
Author
|
I think implementation is ready to review. |
180718c to
b3cef89
Compare
simonhong
commented
Feb 11, 2019
| DVLOG(1) << __func__ << ": Widevine install completed w/o error"; | ||
| // TODO(simonhong): This is very aggresive. | ||
| // Showing relaunch dialog would be better. | ||
| chrome::AttemptRelaunch(); |
Member
Author
There was a problem hiding this comment.
@bbondy I think there are many ways to make user restart.
- just restart after install is success like I did
- change content settings bubble string after install. In this case, blocked image is still visible to user and bubble has different text like
To use widevine, please restart browser. - show relaunch dialog
Which one is best UX? Or please let me know if our UX team has different option.
Member
There was a problem hiding this comment.
@simonhong #2 please. That messaging only for Linux.
Member
Author
d757642 to
f1c7dd1
Compare
0ab6e1f to
6ef8f86
Compare
Widevine install is only downloaded when user accepts the use of widevine from google service via content bubble dialog. Then, it can be used after browser is restarted due to the use of zygote process in linux. Background update is only triggered when brave has different version of widevine with installed version. That means widevine_cdm_version.h included updated widevine version. This new version is also in effect after brwoser restart. We can update version string by changing widevine version in package.json. Of course, we could implement install and update w/o depending on version string. Instead it can be done by fetching latest version from google server. However, this could introduce many upstream file change because upstream code assumes version is not changed in runtime. BraveWidevineBundleManager manages widevine bundle's install state and it uses BraveWidevineBundleUnzipper to unzip downloaded zipped bundle. When installed, contents settings bubble text is changed to "restart".
6ef8f86 to
bf07b8c
Compare
bbondy
approved these changes
Feb 14, 2019
18 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Widevine install is only downloaded when user accepts the use of
widevine from google service via content bubble dialog. Then, it
can be used after browser is restarted due to the use of zygote
process in linux.
Background update is only triggered when brave has different
version of widevine with installed version. That means
widevine_cdm_version.h included updated widevine version.
This new version is also in effect after brwoser restart.
We can update version string by changing widevine version in
package.json.
Of course, we could implement install and update w/o depending on
version string. Instead it can be done by fetching latest version
from google server. However, this could introduce many upstream file
change because upstream code assumes version is not changed in runtime.
BraveWidevineBundleManager manages widevine bundle's install state and
it uses BraveWidevineBundleUnzipper to unzip downloaded zipped bundle.
Fix brave/brave-browser#413
Submitter Checklist:
npm test brave_unit_tests && npm test brave_browser_tests) ongit rebase master(if needed).git rebase -ito squash commits (if needed).Test Plan:
yarn test brave_unit_tests --filter=BraveWidevineBundleManagerTest.*For manual test, please refer to Test Plan in brave/brave-browser#3300
Reviewer Checklist: