Skip to content

Commit

Permalink
Deliver onRenderProcessGone to WebLayer embedders
Browse files Browse the repository at this point in the history
Discussion:
https://docs.google.com/document/d/1M-ocVxRpj43fGrM4mdBazEgcDa0nbWPxD_NxfsPnrkg/edit?ts=5dc96bc9

This CL implements the short-term solution, i.e. only an onRenderProcessGone
callback.

Change-Id: I51cf71975db4d72e940e15b5f2acca6cde3ff4a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1901083
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Pavel Shmakov <pshmakov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714501}
  • Loading branch information
pshmakov authored and Commit Bot committed Nov 12, 2019
1 parent af793f6 commit 8b7b9e2
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ private void visibleUrlChanged(String string) throws RemoteException {
mClient.visibleUrlChanged(string);
}

@CalledByNative
private void onRenderProcessGone() throws RemoteException {
mClient.onRenderProcessGone();
}

@NativeMethods
interface Natives {
long createTabCallbackProxy(TabCallbackProxy proxy, long tab);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,6 @@ interface ITabClient {
void visibleUrlChanged(in String url) = 0;

void onNewTab(in int tabId, in int mode) = 1;

void onRenderProcessGone() = 2;
}
5 changes: 5 additions & 0 deletions weblayer/browser/tab_callback_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ void TabCallbackProxy::DisplayedUrlChanged(const GURL& url) {
Java_TabCallbackProxy_visibleUrlChanged(env, java_observer_, jstring_url);
}

void TabCallbackProxy::OnRenderProcessGone() {
Java_TabCallbackProxy_onRenderProcessGone(AttachCurrentThread(),
java_observer_);
}

static jlong JNI_TabCallbackProxy_CreateTabCallbackProxy(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& proxy,
Expand Down
2 changes: 2 additions & 0 deletions weblayer/browser/tab_callback_proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class TabCallbackProxy : public TabObserver {
// BrowserObserver:
void DisplayedUrlChanged(const GURL& url) override;

void OnRenderProcessGone() override;

private:
Tab* tab_;
base::android::ScopedJavaGlobalRef<jobject> java_observer_;
Expand Down
6 changes: 6 additions & 0 deletions weblayer/browser/tab_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,12 @@ void TabImpl::DidFinishNavigation(
#endif
}

void TabImpl::RenderProcessGone(base::TerminationStatus status) {
for (auto& observer : observers_) {
observer.OnRenderProcessGone();
}
}

void TabImpl::OnExitFullscreen() {
// If |processing_enter_fullscreen_| is true, it means the callback is being
// called while processing EnterFullscreenModeForTab(). WebContents doesn't
Expand Down
1 change: 1 addition & 0 deletions weblayer/browser/tab_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ class TabImpl : public Tab,
// content::WebContentsObserver:
void DidFinishNavigation(
content::NavigationHandle* navigation_handle) override;
void RenderProcessGone(base::TerminationStatus status) override;

// Called from closure supplied to delegate to exit fullscreen.
void OnExitFullscreen();
Expand Down
7 changes: 7 additions & 0 deletions weblayer/public/java/org/chromium/weblayer/Tab.java
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,13 @@ public void onNewTab(int tabId, int mode) {
assert tab.getBrowser() == getBrowser();
mNewTabCallback.onNewTab(tab, mode);
}

@Override
public void onRenderProcessGone() {
for (TabCallback callback : mCallbacks) {
callback.onRenderProcessGone();
}
}
}

private static final class DownloadCallbackClientImpl extends IDownloadCallbackClient.Stub {
Expand Down
6 changes: 6 additions & 0 deletions weblayer/public/java/org/chromium/weblayer/TabCallback.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,10 @@ public abstract class TabCallback {
* @param url The new user-visible url.
*/
public void onVisibleUrlChanged(@NonNull Uri url) {}

/**
* Triggered when the render process dies, either due to crash or killed by the system to
* reclaim memory.
*/
public void onRenderProcessGone() {}
}
4 changes: 4 additions & 0 deletions weblayer/public/tab_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ class TabObserver {
// The URL bar should be updated to |url|.
virtual void DisplayedUrlChanged(const GURL& url) {}

// Triggered when the render process dies, either due to crash or killed by the system to
// reclaim memory.
virtual void OnRenderProcessGone() {}

protected:
virtual ~TabObserver() {}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,18 @@
import org.junit.runner.RunWith;

import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.content_public.browser.test.util.Criteria;
import org.chromium.content_public.browser.test.util.CriteriaHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.weblayer.Tab;
import org.chromium.weblayer.TabCallback;
import org.chromium.weblayer.shell.InstrumentationActivity;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.TimeoutException;

/**
* Tests that TabCallback methods are invoked as expected.
Expand Down Expand Up @@ -71,14 +74,33 @@ public void testLoadEvents() {
String startupUrl = "about:blank";
InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl(startupUrl);

Callback calllback = new Callback();
Callback callback = new Callback();
TestThreadUtils.runOnUiThreadBlocking(
() -> { activity.getTab().registerTabCallback(calllback); });
() -> { activity.getTab().registerTabCallback(callback); });

String url = "data:text,foo";
mActivityTestRule.navigateAndWait(url);

/* Verify that the visible URL changes to the target. */
calllback.visibleUrlChangedCallback.waitUntilValueObserved(url);
callback.visibleUrlChangedCallback.waitUntilValueObserved(url);
}

@Test
@SmallTest
public void testOnRenderProcessGone() throws TimeoutException {
InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl("about:blank");
CallbackHelper callbackHelper = new CallbackHelper();
TestThreadUtils.runOnUiThreadBlocking(() -> {
Tab tab = activity.getTab();
TabCallback callback = new TabCallback() {
@Override
public void onRenderProcessGone() {
callbackHelper.notifyCalled();
}
};
tab.registerTabCallback(callback);
tab.getNavigationController().navigate(Uri.parse("chrome://crash"));
});
callbackHelper.waitForFirst();
}
}

0 comments on commit 8b7b9e2

Please sign in to comment.