[USS] Impelementation for GetData and GetAllData

components/autofill/core/browser/webdata/autocomplete_sync_bridge.h

 // Copyright 2016 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 COMPONENTS_AUTOFILL_CORE_BROWSER_WEBDATA_AUTOCOMPLETE_SYNC_BRIDGE_H_
 #define COMPONENTS_AUTOFILL_CORE_BROWSER_WEBDATA_AUTOCOMPLETE_SYNC_BRIDGE_H_
 
 #include "base/scoped_observer.h"
 #include "base/supports_user_data.h"
 #include "base/threading/non_thread_safe.h"
@@ skipping 47 matching lines @@
   std::string GetClientTag(const syncer::EntityData& entity_data) override;
   // Generate a string key uniquely identifying |entity_data| in the context of
   // local storage. The format, which should stay the same, is $name|$value"
   // where $name and $value are escaped.
   std::string GetStorageKey(const syncer::EntityData& entity_data) override;
 
   // AutofillWebDataServiceObserverOnDBThread implementation.
   void AutofillEntriesChanged(const AutofillChangeList& changes) override;
 
  private:
+  friend class AutocompleteSyncBridgeTest;

[vabr (Chromium), 2016/12/21 08:48:07]

Please do not friend the test. Tests should only depend on the public API,
making them depend on the internals makes them fragile. I added some concrete
suggestions below for how to keep your current test without the need to friend
it with the bridge.

[Gang Wu, 2016/12/22 01:33:53]

Done.

+
   // Returns the table associated with the |web_data_backend_|.
   AutofillTable* GetAutofillTable() const;
 
+  std::string GetStorageKeyFromAutofillEntry(
+      const autofill::AutofillEntry& entry);
+
+  static std::string FormatStorageKey(const std::string& name,
+                                      const std::string& value);
+
+  static AutofillEntry CreateAutofillEntry(

[vabr (Chromium), 2016/12/21 08:48:07]

I can only see this used in the test. Could you define it as an anonymous
function there?

[skym, 2016/12/21 21:50:41]

I think moving this function to the test file would be short sighted. We're
going to need this function for the bridge implementation. Maybe this could
simply be public instead of private? I don't think it would hurt to unittest
this function directly. I think converting between specifics and AutofillEntry
and/or AutofillKey objects is essentially core functionality the bridge
performs.

[Gang Wu, 2016/12/22 01:33:53]

you are right, we need this function later on, even I move this one to unittest,
we will move it back when sync data from sync server to autofill. So change it
to public.

[Gang Wu, 2016/12/22 01:33:53]

Done.

+      const sync_pb::AutofillSpecifics& autofill_specifics);
+
   base::ThreadChecker thread_checker_;
 
   // AutocompleteSyncBridge is owned by |web_data_backend_| through
   // SupportsUserData, so it's guaranteed to outlive |this|.
   AutofillWebDataBackend* const web_data_backend_;
 
   ScopedObserver<AutofillWebDataBackend, AutocompleteSyncBridge>
       scoped_observer_;
 };
 
 }  // namespace autofill
 
 #endif  // COMPONENTS_AUTOFILL_CORE_BROWSER_WEBDATA_AUTOCOMPLETE_SYNC_BRIDGE_H_