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

Add support for market:// urls #22091

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -63,6 +63,8 @@ pub trait HostTrait {
fn on_load_ended(&self);
/// Page title has changed.
fn on_title_changed(&self, title: String);
/// Allow Navigation.
fn on_allow_navigation(&self, url: String) -> bool;
/// Page URL has changed.
fn on_url_changed(&self, url: String);
/// Back/forward state has changed.
@@ -364,8 +366,12 @@ impl ServoGlue {
let title = format!("{} - Servo", title);
self.callbacks.host_callbacks.on_title_changed(title);
},
EmbedderMsg::AllowNavigation(_url, response_chan) => {
if let Err(e) = response_chan.send(true) {
EmbedderMsg::AllowNavigation(url, response_chan) => {
let data: bool = self
.callbacks
.host_callbacks
.on_allow_navigation(url.to_string());
if let Err(e) = response_chan.send(data) {
warn!("Failed to send allow_navigation() response: {}", e);
};
},
@@ -35,6 +35,7 @@ pub struct CHostCallbacks {
pub on_load_started: extern "C" fn(),
pub on_load_ended: extern "C" fn(),
pub on_title_changed: extern "C" fn(title: *const c_char),
pub on_allow_navigation: extern "C" fn(url: *const c_char) -> bool,
pub on_url_changed: extern "C" fn(url: *const c_char),
pub on_history_changed: extern "C" fn(can_go_back: bool, can_go_forward: bool),
pub on_animating_changed: extern "C" fn(animating: bool),
@@ -302,6 +303,14 @@ impl HostTrait for HostCallbacks {
(self.0.on_title_changed)(title_ptr);
}

fn on_allow_navigation(&self, url: String) -> bool {
debug!("on_allow_navigation");
let url = CString::new(url).expect("Can't create string");
let url_ptr = url.as_ptr();
mem::forget(url);
(self.0.on_allow_navigation)(url_ptr)
}

fn on_url_changed(&self, url: String) {
debug!("on_url_changed");
let url = CString::new(url).expect("Can't create string");
@@ -395,6 +395,26 @@ impl HostTrait for HostCallbacks {
.unwrap();
}

fn on_allow_navigation(&self, url: String) -> bool {
debug!("on_allow_navigation");
let env = self.jvm.get_env().unwrap();
let s = match new_string(&env, &url) {
Ok(s) => s,
Err(_) => return false,
};
let s = JValue::from(JObject::from(s));
let allow = env.call_method(
self.callbacks.as_obj(),
"onAllowNavigation",
"(Ljava/lang/String;)Z",
&[s],
);
match allow {
Ok(allow) => return allow.z().unwrap(),
Err(_) => return true,
}
}

fn on_url_changed(&self, url: String) {
debug!("on_url_changed");
let env = self.jvm.get_env().unwrap();
@@ -20,6 +20,7 @@
import android.widget.EditText;
import android.widget.ProgressBar;
import android.widget.TextView;
import android.util.Log;

import org.mozilla.servoview.ServoView;
import org.mozilla.servoview.Servo;
@@ -162,6 +163,19 @@ public void onHistoryChanged(boolean canGoBack, boolean canGoForward) {
mCanGoBack = canGoBack;
}

@Override
public boolean onAllowNavigation(String url) {
if (url.startsWith("market://")) {
try {
startActivity(new Intent(Intent.ACTION_VIEW, Uri.parse(url)));
return false;
} catch (Exception e) {
Log.e("onAllowNavigation", e.toString());

This comment has been minimized.

Copy link
@akshitkrnagpal

akshitkrnagpal Nov 2, 2018

Author Contributor

@paulrouget
Do we need to do something here?

To my knowledge, this exception will be thrown on 2 cases. If there is no app to handle the URL (play store is not installed) or there is a problem in parsing URL.

This comment has been minimized.

Copy link
@paulrouget

paulrouget Nov 5, 2018

Contributor

Printing a warning and letting it return true sounds about right.

This comment has been minimized.

Copy link
@akshitkrnagpal

akshitkrnagpal Nov 5, 2018

Author Contributor

I'll update this with Log.w. 👍

This comment has been minimized.

Copy link
@paulrouget

paulrouget Nov 5, 2018

Contributor

error or warning, both fine.

This comment has been minimized.

Copy link
@akshitkrnagpal

akshitkrnagpal Nov 5, 2018

Author Contributor

I guess then this is fine.

}
}
return true;
}

public void onRedrawing(boolean redrawing) {
if (redrawing) {
mIdleText.setText("LOOP");
@@ -128,6 +128,8 @@ public void suspend(boolean suspended) {
}

public interface Client {
boolean onAllowNavigation(String url);

void onLoadStarted();

void onLoadEnded();
@@ -191,6 +193,10 @@ public void onAnimatingChanged(boolean animating) {
mRunCallback.inGLThread(() -> mGfxCb.animationStateChanged(animating));
}

public boolean onAllowNavigation(String url) {
return mClient.onAllowNavigation(url);

This comment has been minimized.

Copy link
@paulrouget

paulrouget Nov 5, 2018

Contributor

So this works without wrapping the call to mClient.onAllowNavigation into a mRunCallback.inUIThread? Maybe it's better to wrap it, we are not sure the code in mClient.onAllowNavigation will always work properly in the GL thread.

This comment has been minimized.

Copy link
@akshitkrnagpal

akshitkrnagpal Nov 5, 2018

Author Contributor

Maybe it's better to wrap it, we are not sure the code in mClient.onAllowNavigation will always work properly in the GL thread.

The problem here is we cannot return value (in this case boolean) from Runnable. We'll need a Callable interface for that. I am not entirely sure how to do that here. Since methods inGLThread and inUIThread take Runnable as arguments.

If you can give me a little direction I would love to work towards that. 😄

This comment has been minimized.

Copy link
@paulrouget

paulrouget Nov 5, 2018

Contributor

Good catch. Let's keep it that way then.

}

public void onLoadStarted() {
mRunCallback.inUIThread(() -> mClient.onLoadStarted());
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.