Refactor BrowsingDataRemover to mock out WebappRegistry.

chrome/browser/browsing_data/browsing_data_remover_unittest.cc

Index: chrome/browser/browsing_data/browsing_data_remover_unittest.cc
diff --git a/chrome/browser/browsing_data/browsing_data_remover_unittest.cc b/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
index 223fd0cd9a0824d4ae4aa76db0df698b3a80c376..2b44b9d664d4466b64688cfe5893fa96c3feb7bd 100644
--- a/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
+++ b/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
@@ -76,6 +76,10 @@
 #include "ui/gfx/favicon_size.h"
 #include "url/origin.h"
 
+#if BUILDFLAG(ANDROID_JAVA_UI)
+#include "chrome/browser/android/webapps/webapp_registry.h"
+#endif
+
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/login/users/mock_user_manager.h"
 #include "chrome/browser/chromeos/login/users/scoped_user_manager_enabler.h"
@@ -260,6 +264,18 @@ class TestStoragePartition : public StoragePartition {
   DISALLOW_COPY_AND_ASSIGN(TestStoragePartition);
 };
 
+#if BUILDFLAG(ANDROID_JAVA_UI)
+class TestWebappRegistry : public WebappRegistry {
+ public:
+  TestWebappRegistry() : WebappRegistry() { }
+
+  void UnregisterWebapps(const base::Closure& callback) override {
+    // Mocks out a JNI call and runs the callback.
+    callback.Run();

[gone, 2016/03/10 00:18:06]

Should this be a delayed task, like with the real call?  This will immediately
run the callback on the same thread.

[dominickn, 2016/03/10 00:30:45]

My reasoning was that I wanted to block as little as possible in C++ unit tests.
This mocking should only happen in circumstances where the Java calls won't
work. Do you think this is valid?

[gone, 2016/03/10 00:45:07]

Although the Java calls won't work, I'd normally argue that getting your mocked
class to work as similarly as possible could catch different behavior.  If, for
example, the real code did something non-trivial immediately when the mock
callback was called, this could make things occur in a different order than in
the real world.

In this case, however, I'm not sure there's much benefit with the current code
path because the BrowsingDataRemover is already waiting for other things
asynchronously.

So, your call.  I can swing either way.

+  }
+};
+#endif
+
 // Custom matcher to test the equivalence of two URL filters. Since those are
 // blackbox predicates, we can only approximate the equivalence by testing
 // whether the filter give the same answer for several URLs. This is currently
@@ -942,6 +958,13 @@ class BrowsingDataRemoverTest : public testing::Test {
         BrowsingDataRemover::RegisterOnBrowsingDataRemovedCallback(
             base::Bind(&BrowsingDataRemoverTest::NotifyWithDetails,
                        base::Unretained(this)));
+
+#if BUILDFLAG(ANDROID_JAVA_UI)
+    BrowsingDataRemover* remover =
+        BrowsingDataRemoverFactory::GetForBrowserContext(profile_.get());
+    remover->OverrideWebappRegistryForTesting(
+        scoped_ptr<WebappRegistry>(new TestWebappRegistry()));
+#endif
   }
 
   ~BrowsingDataRemoverTest() override {}