Skip to content

Commit

Permalink
weblayer: adds Profile.destroyAndDeleteDataFromDiskSoon
Browse files Browse the repository at this point in the history
This method immediately marks the profile for deletion, and calls
destroyAndDeleteDataFromDisk() when safe. Safe means no more browsers
reference the profile.

I kept most of the logic for this in the Java side, as I don't think
the C++ side needs this.

BUG=1142989
TEST=testDestroyAndDeleteDataFromDiskSoonWhenInUse and
    DestroyFromOnBrowserRemoved

Change-Id: Ic5322022ab49a515d6e0319159ff79c91249db7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2519836
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824238}
  • Loading branch information
Scott Violet authored and Commit Bot committed Nov 5, 2020
1 parent d593e5c commit 6acf9a2
Show file tree
Hide file tree
Showing 18 changed files with 420 additions and 9 deletions.
2 changes: 2 additions & 0 deletions weblayer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,8 @@ source_set("weblayer_lib_base") {
"browser/browser_controls_navigation_state_handler.cc",
"browser/browser_controls_navigation_state_handler.h",
"browser/browser_controls_navigation_state_handler_delegate.h",
"browser/browser_list_proxy.cc",
"browser/browser_list_proxy.h",
"browser/content_view_render_view.cc",
"browser/content_view_render_view.h",
"browser/devtools_manager_delegate_android.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import android.net.Uri;
import android.webkit.ValueCallback;

import androidx.fragment.app.FragmentManager;
import androidx.test.filters.SmallTest;

import org.junit.Assert;
Expand Down Expand Up @@ -88,6 +89,28 @@ public void testDestroyAndDeleteDataFromDiskWhenInUse() {
}
}

@Test
@SmallTest
@MinWebLayerVersion(88)
public void testDestroyAndDeleteDataFromDiskSoonWhenInUse() throws Exception {
WebLayer weblayer = mActivityTestRule.getWebLayer();
InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl("about:blank");
final CallbackHelper callbackHelper = new CallbackHelper();
Profile profile =
TestThreadUtils.runOnUiThreadBlocking(() -> activity.getBrowser().getProfile());
TestThreadUtils.runOnUiThreadBlocking(() -> {
profile.destroyAndDeleteDataFromDiskSoon(callbackHelper::notifyCalled);
FragmentManager fm = activity.getSupportFragmentManager();
fm.beginTransaction()
.remove(fm.getFragments().get(0))
.runOnCommit(callbackHelper::notifyCalled)
.commit();
});
callbackHelper.waitForCallback(0, 2);
Collection<Profile> profiles = getAllProfiles();
Assert.assertFalse(profiles.contains(profile));
}

@Test
@SmallTest
public void testDestroyAndDeleteDataFromDisk() throws Exception {
Expand Down
4 changes: 4 additions & 0 deletions weblayer/browser/browser_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ class BrowserImpl : public Browser {
#if defined(OS_ANDROID)
bool CompositorHasSurface();

base::android::ScopedJavaGlobalRef<jobject> java_browser() {
return java_impl_;
}

void AddTab(JNIEnv* env,
long native_tab);
base::android::ScopedJavaLocalRef<jobjectArray> GetTabs(JNIEnv* env);
Expand Down
22 changes: 20 additions & 2 deletions weblayer/browser/browser_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
#include "weblayer/browser/browser_impl.h"
#include "weblayer/browser/browser_list_observer.h"

#if defined(OS_ANDROID)
#include "weblayer/browser/browser_list_proxy.h"
#endif

namespace weblayer {

// static
Expand All @@ -33,9 +37,18 @@ void BrowserList::RemoveObserver(BrowserListObserver* observer) {
observers_.RemoveObserver(observer);
}

BrowserList::BrowserList() = default;
BrowserList::BrowserList() {
#if defined(OS_ANDROID)
browser_list_proxy_ = std::make_unique<BrowserListProxy>();
AddObserver(browser_list_proxy_.get());
#endif
}

BrowserList::~BrowserList() = default;
BrowserList::~BrowserList() {
#if defined(OS_ANDROID)
RemoveObserver(browser_list_proxy_.get());
#endif
}

void BrowserList::AddBrowser(BrowserImpl* browser) {
DCHECK(!browsers_.contains(browser));
Expand All @@ -44,6 +57,8 @@ void BrowserList::AddBrowser(BrowserImpl* browser) {
DCHECK(!browser->fragment_resumed());
#endif
browsers_.insert(browser);
for (BrowserListObserver& observer : observers_)
observer.OnBrowserCreated(browser);
}

void BrowserList::RemoveBrowser(BrowserImpl* browser) {
Expand All @@ -53,6 +68,9 @@ void BrowserList::RemoveBrowser(BrowserImpl* browser) {
DCHECK(!browser->fragment_resumed());
#endif
browsers_.erase(browser);

for (BrowserListObserver& observer : observers_)
observer.OnBrowserDestroyed(browser);
}

#if defined(OS_ANDROID)
Expand Down
7 changes: 7 additions & 0 deletions weblayer/browser/browser_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ namespace weblayer {
class BrowserImpl;
class BrowserListObserver;

#if defined(OS_ANDROID)
class BrowserListProxy;
#endif

// Tracks the set of browsers.
class BrowserList {
public:
Expand Down Expand Up @@ -49,6 +53,9 @@ class BrowserList {

base::flat_set<BrowserImpl*> browsers_;
base::ObserverList<BrowserListObserver> observers_;
#if defined(OS_ANDROID)
std::unique_ptr<BrowserListProxy> browser_list_proxy_;
#endif
};

} // namespace weblayer
Expand Down
6 changes: 6 additions & 0 deletions weblayer/browser/browser_list_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

namespace weblayer {

class Browser;

class BrowserListObserver : public base::CheckedObserver {
public:
#if defined(OS_ANDROID)
Expand All @@ -18,6 +20,10 @@ class BrowserListObserver : public base::CheckedObserver {
void OnHasAtLeastOneResumedBrowserStateChanged(bool new_value) {}
#endif

virtual void OnBrowserCreated(Browser* browser) {}

virtual void OnBrowserDestroyed(Browser* browser) {}

protected:
~BrowserListObserver() override = default;
};
Expand Down
36 changes: 36 additions & 0 deletions weblayer/browser/browser_list_proxy.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "weblayer/browser/browser_list_proxy.h"

#include "base/android/jni_android.h"
#include "weblayer/browser/browser_impl.h"
#include "weblayer/browser/browser_list.h"
#include "weblayer/browser/java/jni/BrowserList_jni.h"

namespace weblayer {

BrowserListProxy::BrowserListProxy()
: java_browser_list_(Java_BrowserList_createBrowserList(
base::android::AttachCurrentThread())) {}

BrowserListProxy::~BrowserListProxy() = default;

void BrowserListProxy::OnBrowserCreated(Browser* browser) {
Java_BrowserList_onBrowserCreated(
base::android::AttachCurrentThread(), java_browser_list_,
static_cast<BrowserImpl*>(browser)->java_browser());
}

void BrowserListProxy::OnBrowserDestroyed(Browser* browser) {
Java_BrowserList_onBrowserDestroyed(
base::android::AttachCurrentThread(), java_browser_list_,
static_cast<BrowserImpl*>(browser)->java_browser());
}

static void JNI_BrowserList_CreateBrowserList(JNIEnv* env) {
BrowserList::GetInstance();
}

} // namespace weblayer
34 changes: 34 additions & 0 deletions weblayer/browser/browser_list_proxy.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef WEBLAYER_BROWSER_BROWSER_LIST_PROXY_H_
#define WEBLAYER_BROWSER_BROWSER_LIST_PROXY_H_

#include <jni.h>

#include "base/android/scoped_java_ref.h"
#include "weblayer/browser/browser_list_observer.h"

namespace weblayer {

// Owns the Java BrowserList implementation and funnels all BrowserListObserver
// calls to the java side.
class BrowserListProxy : public BrowserListObserver {
public:
BrowserListProxy();
BrowserListProxy(const BrowserListProxy&) = delete;
BrowserListProxy& operator=(const BrowserListProxy&) = delete;
~BrowserListProxy() override;

// BrowserListObserver:
void OnBrowserCreated(Browser* browser) override;
void OnBrowserDestroyed(Browser* browser) override;

private:
base::android::ScopedJavaGlobalRef<jobject> java_browser_list_;
};

} // namespace weblayer

#endif // WEBLAYER_BROWSER_BROWSER_LIST_PROXY_H_
4 changes: 4 additions & 0 deletions weblayer/browser/java/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ android_library("java") {
"org/chromium/weblayer_private/BrowserControlsContainerView.java",
"org/chromium/weblayer_private/BrowserFragmentImpl.java",
"org/chromium/weblayer_private/BrowserImpl.java",
"org/chromium/weblayer_private/BrowserList.java",
"org/chromium/weblayer_private/BrowserListObserver.java",
"org/chromium/weblayer_private/BrowserViewController.java",
"org/chromium/weblayer_private/ChildProcessServiceImpl.java",
"org/chromium/weblayer_private/ContactsPickerAdapter.java",
Expand Down Expand Up @@ -328,6 +330,7 @@ generate_jni("jni") {
"org/chromium/weblayer_private/AutocompleteSchemeClassifierImpl.java",
"org/chromium/weblayer_private/BrowserControlsContainerView.java",
"org/chromium/weblayer_private/BrowserImpl.java",
"org/chromium/weblayer_private/BrowserList.java",
"org/chromium/weblayer_private/ContentViewRenderView.java",
"org/chromium/weblayer_private/CookieManagerImpl.java",
"org/chromium/weblayer_private/DownloadCallbackProxy.java",
Expand Down Expand Up @@ -447,6 +450,7 @@ android_aidl("aidl") {
"org/chromium/weblayer_private/interfaces/IObjectWrapper.aidl",
"org/chromium/weblayer_private/interfaces/IPrerenderController.aidl",
"org/chromium/weblayer_private/interfaces/IProfile.aidl",
"org/chromium/weblayer_private/interfaces/IProfileClient.aidl",
"org/chromium/weblayer_private/interfaces/IRemoteFragment.aidl",
"org/chromium/weblayer_private/interfaces/IRemoteFragmentClient.aidl",
"org/chromium/weblayer_private/interfaces/ISiteSettingsFragment.aidl",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.weblayer_private;

import androidx.annotation.NonNull;

import org.chromium.base.ObserverList;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods;

/**
* Tracks the set of Browsers.
*/
@JNINamespace("weblayer")
public final class BrowserList {
private static BrowserList sInstance;
private final ObserverList<BrowserListObserver> mObservers;

@CalledByNative
private static BrowserList createBrowserList() {
// The native side should call this only once.
assert sInstance == null;
sInstance = new BrowserList();
return sInstance;
}

@NonNull
public static BrowserList getInstance() {
// The native side creates this early on. It should never be null.
if (sInstance == null) {
BrowserListJni.get().createBrowserList();
assert sInstance != null;
}
return sInstance;
}

private BrowserList() {
mObservers = new ObserverList<>();
}

public void addObserver(BrowserListObserver o) {
mObservers.addObserver(o);
}

public void removeObserver(BrowserListObserver o) {
mObservers.removeObserver(o);
}

@CalledByNative
private void onBrowserCreated(BrowserImpl browser) {
for (BrowserListObserver observer : mObservers) {
observer.onBrowserCreated(browser);
}
}

@CalledByNative
private void onBrowserDestroyed(BrowserImpl browser) {
for (BrowserListObserver observer : mObservers) {
observer.onBrowserDestroyed(browser);
}
}

@NativeMethods
interface Natives {
void createBrowserList();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.weblayer_private;

/**
* Notified of changes to BrowserList.
*/
public interface BrowserListObserver {
void onBrowserCreated(BrowserImpl browser);
void onBrowserDestroyed(BrowserImpl browser);
}
Loading

0 comments on commit 6acf9a2

Please sign in to comment.