dpwa: Add unit test for backward compatibility.

BackwardCompatibility_WebAppWithOnlyRequiredFields tests the reading of the
minimal proto data with `required` fields only.

Any new fields in web_app.proto should be `optional` since the BMO
project launch:
we need backward compatibility for any previous user's data state.

If `required` field gets added into web_app.proto, this test fails.

It is still possible to add required fields to optional/repeated
protos, like WebAppShortcutsMenuItemInProto, and this test will pass,
so it's won't catch 100% of new required fields (only minimal).

Bug: 1104718
Change-Id: I1a76f19df6f9f5f81c838fc029467f1cd48c10a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2302977
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Chris Mumford <cmumford@google.com>
Cr-Commit-Position: refs/heads/master@{#791092}

chrome/browser/web_applications/test/test_web_app_database_factory.cc

@@ -6,6 +6,7 @@
 
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
+#include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/proto/web_app.pb.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/web_applications/web_app_database.h"
@@ -53,22 +54,26 @@ Registry TestWebAppDatabaseFactory::ReadRegistry() const {
 std::set<AppId> TestWebAppDatabaseFactory::ReadAllAppIds() const {
   std::set<AppId> app_ids;
 
-  auto registry = ReadRegistry();
-  for (auto& kv : registry)
+  Registry registry = ReadRegistry();
+  for (Registry::value_type& kv : registry)
     app_ids.insert(kv.first);
 
   return app_ids;
 }
 
-void TestWebAppDatabaseFactory::WriteRegistry(const Registry& registry) {
+void TestWebAppDatabaseFactory::WriteProtos(
+    const std::vector<std::unique_ptr<WebAppProto>>& protos) {
   base::RunLoop run_loop;
 
-  auto write_batch = store_->CreateWriteBatch();
+  std::unique_ptr<syncer::ModelTypeStore::WriteBatch> write_batch =
+      store_->CreateWriteBatch();
 
-  for (auto& kv : registry) {
-    const WebApp* app = kv.second.get();
-    auto proto = WebAppDatabase::CreateWebAppProto(*app);
-    write_batch->WriteData(app->app_id(), proto->SerializeAsString());
+  for (const std::unique_ptr<WebAppProto>& proto : protos) {
+    GURL launch_url(proto->sync_data().launch_url());
+    DCHECK(!launch_url.is_empty());
+    DCHECK(launch_url.is_valid());
+    AppId app_id = GenerateAppIdFromURL(launch_url);
+    write_batch->WriteData(app_id, proto->SerializeAsString());
   }
 
   store_->CommitWriteBatch(
@@ -82,4 +87,16 @@ void TestWebAppDatabaseFactory::WriteRegistry(const Registry& registry) {
   run_loop.Run();
 }
 
+void TestWebAppDatabaseFactory::WriteRegistry(const Registry& registry) {
+  std::vector<std::unique_ptr<WebAppProto>> protos;
+  for (const Registry::value_type& kv : registry) {
+    const WebApp* app = kv.second.get();
+    std::unique_ptr<WebAppProto> proto =
+        WebAppDatabase::CreateWebAppProto(*app);
+    protos.push_back(std::move(proto));
+  }
+
+  WriteProtos(protos);
+}
+
 }  // namespace web_app

chrome/browser/web_applications/test/test_web_app_database_factory.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 #include <set>
+#include <vector>
 
 #include "base/macros.h"
 #include "chrome/browser/web_applications/components/web_app_id.h"
@@ -19,6 +20,8 @@ class ModelTypeStore;
 
 namespace web_app {
 
+class WebAppProto;
+
 // Requires base::MessageLoop message_loop_ in test fixture. Reason:
 // InMemoryStore needs a SequencedTaskRunner.
 // MessageLoop ctor calls MessageLoop::SetThreadTaskRunnerHandle().
@@ -36,6 +39,7 @@ class TestWebAppDatabaseFactory : public AbstractWebAppDatabaseFactory {
 
   std::set<AppId> ReadAllAppIds() const;
 
+  void WriteProtos(const std::vector<std::unique_ptr<WebAppProto>>& protos);
   void WriteRegistry(const Registry& registry);
 
  private:

chrome/browser/web_applications/web_app_database_unittest.cc

@@ -23,6 +23,7 @@
 #include "chrome/browser/web_applications/test/test_web_app_registry_controller.h"
 #include "chrome/browser/web_applications/test/web_app_test.h"
 #include "chrome/browser/web_applications/web_app.h"
+#include "chrome/browser/web_applications/web_app_proto_utils.h"
 #include "chrome/browser/web_applications/web_app_registrar.h"
 #include "chrome/browser/web_applications/web_app_registry_update.h"
 #include "chrome/browser/web_applications/web_app_sync_bridge.h"
@@ -388,6 +389,68 @@ TEST_F(WebAppDatabaseTest, OpenDatabaseAndReadRegistry) {
   EXPECT_TRUE(IsRegistryEqual(mutable_registrar().registry(), registry));
 }
 
+TEST_F(WebAppDatabaseTest, BackwardCompatibility_WebAppWithOnlyRequiredFields) {
+  const GURL launch_url{"https://example.com/"};
+  const AppId app_id = GenerateAppIdFromURL(launch_url);
+  const std::string name = "App Name";
+  const auto user_display_mode = DisplayMode::kBrowser;
+  const bool is_locally_installed = true;
+
+  std::vector<std::unique_ptr<WebAppProto>> protos;
+
+  // Create a proto with |required| only fields.
+  // Do not add new fields in this test: any new fields should be |optional|.
+  auto proto = std::make_unique<WebAppProto>();
+  {
+    sync_pb::WebAppSpecifics sync_proto;
+    sync_proto.set_launch_url(launch_url.spec());
+    sync_proto.set_user_display_mode(
+        ToWebAppSpecificsUserDisplayMode(user_display_mode));
+    *(proto->mutable_sync_data()) = std::move(sync_proto);
+  }
+
+  proto->set_name(name);
+  proto->set_is_locally_installed(is_locally_installed);
+
+  proto->mutable_sources()->set_system(false);
+  proto->mutable_sources()->set_policy(false);
+  proto->mutable_sources()->set_web_app_store(false);
+  proto->mutable_sources()->set_sync(true);
+  proto->mutable_sources()->set_default_(false);
+
+  if (IsChromeOs()) {
+    proto->mutable_chromeos_data()->set_show_in_launcher(false);
+    proto->mutable_chromeos_data()->set_show_in_search(false);
+    proto->mutable_chromeos_data()->set_show_in_management(false);
+    proto->mutable_chromeos_data()->set_is_disabled(true);
+  }
+
+  protos.push_back(std::move(proto));
+  database_factory().WriteProtos(protos);
+
+  // Read the registry: the proto parsing may fail while reading the proto
+  // above.
+  controller().Init();
+
+  const WebApp* app = registrar().GetAppById(app_id);
+  EXPECT_EQ(app_id, app->app_id());
+  EXPECT_EQ(launch_url, app->launch_url());
+  EXPECT_EQ(name, app->name());
+  EXPECT_EQ(user_display_mode, app->user_display_mode());
+  EXPECT_EQ(is_locally_installed, app->is_locally_installed());
+  EXPECT_TRUE(app->IsSynced());
+  EXPECT_FALSE(app->IsDefaultApp());
+
+  if (IsChromeOs()) {
+    EXPECT_FALSE(app->chromeos_data()->show_in_launcher);
+    EXPECT_FALSE(app->chromeos_data()->show_in_search);
+    EXPECT_FALSE(app->chromeos_data()->show_in_management);
+    EXPECT_TRUE(app->chromeos_data()->is_disabled);
+  } else {
+    EXPECT_FALSE(app->chromeos_data().has_value());
+  }
+}
+
 TEST_F(WebAppDatabaseTest, WebAppWithoutOptionalFields) {
   controller().Init();