Skip to content
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

H5vccPlatformService #4471

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

zhongqiliang
Copy link
Contributor

@zhongqiliang zhongqiliang commented Nov 21, 2024

This PR contains a few critical changes.

  1. It fixed the race condition of injecting the java bridge object and loading the url from shell.
  2. The pattern to polyfill the embeded javascript in the APK binary before Kabuki loads won't work, see context on b/379731250, as a result, we will move the javascript file chrobalt_preload.js and its sub modules on to Kabuki codebase. We will create a backlog bug for this when it is submitted.
  3. H5vccPlatformService synchronize communicate works. I used ClientLogInfo to test H5vccPlatformService locally, this platform service is not used on production, I would submit my prototype code into the repository for checking the history in the future.

b/379731250
b/379184982
b/378571210

@zhongqiliang zhongqiliang marked this pull request as ready for review November 21, 2024 21:58
@zhongqiliang zhongqiliang requested review from yell0wd0g, johnxwork, jasonzhangxx and kaidokert and removed request for yell0wd0g and johnxwork November 21, 2024 22:13
mShellManager.launchShell(shellUrl);

// Load an empty page to let shell create WebContents.
mShellManager.launchShell("");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a more graceful way to do the init? You could add a TODO and leave the idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the right way to do it. Alternatively, we will need to add code in content shell package here. https://source.chromium.org/chromium/chromium/src/+/main:content/shell/browser/shell.cc;l=230;drc=5d0194cbc9cddc2ad096da32f16d7d1eac5d4a34

@@ -14,16 +14,20 @@

package dev.cobalt.coat;

import static dev.cobalt.util.Log.TAG;
// import static dev.cobalt.util.Log.TAG;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we able to remove these commented code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but I prefer leave it here, because it is needed for platform service asynchronize response, I will uncomment it in the next PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants