Move ShouldFilterAutofillResult from ChromePasswordManagerClient to PasswordManager

components/password_manager/sync/browser/sync_store_result_filter.cc

Index: components/password_manager/sync/browser/sync_store_result_filter.cc
diff --git a/components/password_manager/sync/browser/sync_store_result_filter.cc b/components/password_manager/sync/browser/sync_store_result_filter.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0909038a90d03b602324f7169560fdd69dd9eebb
--- /dev/null
+++ b/components/password_manager/sync/browser/sync_store_result_filter.cc
@@ -0,0 +1,91 @@
+// Copyright 2015 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 "components/password_manager/sync/browser/sync_store_result_filter.h"
+
+#include "base/command_line.h"
+#include "base/metrics/field_trial.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/strings/utf_string_conversions.h"
+#include "components/password_manager/core/browser/password_manager_client.h"
+#include "components/password_manager/core/common/password_manager_switches.h"
+#include "google_apis/gaia/gaia_urls.h"
+#include "net/base/url_util.h"
+
+namespace password_manager {
+
+namespace {
+
+// Returns true if the last loaded page was for transactional re-auth on a
+// Google property.
+bool LastLoadWasTransactionalReauthPage(const GURL& last_load_url) {
+  if (last_load_url.GetOrigin() !=
+      GaiaUrls::GetInstance()->gaia_url().GetOrigin())
+    return false;
+
+  // "rart" is the transactional reauth paramter.
+  std::string ignored_value;
+  return net::GetValueForKeyInQuery(last_load_url, "rart", &ignored_value);
+}
+
+}  // namespace
+
+SyncStoreResultFilter::SyncStoreResultFilter(
+    const PasswordManagerClient* client)
+    : client_(client),
+      autofill_sync_state_(SetUpAutofillSyncState()),
+      sync_credential_was_filtered_(false) {}
+
+SyncStoreResultFilter::~SyncStoreResultFilter() {
+  UMA_HISTOGRAM_BOOLEAN("PasswordManager.SyncCredentialFiltered",
+                        sync_credential_was_filtered_);
+}
+
+bool SyncStoreResultFilter::ShouldIgnore(const autofill::PasswordForm& form) {
+  // TODO(vabr) Move IsSyncAccountCredential here.
+  if (!client_->IsSyncAccountCredential(base::UTF16ToUTF8(form.username_value),
+                                        form.signon_realm))

[engedy, 2015/08/04 10:11:29]

nit: Include password_form.h directly, there seems to be no reasons why it would
be necessarily pulled in by any other includes

[vabr (Chromium), 2015/08/04 12:21:25]

The header for the interface includes it, no? Anyway, with #ifdef guards I don't
think it hurts to include it in the .cc file. Until we have a tool to sanitise
#includes, there will be redundancy and other mess anyway. :)
Done.

[engedy, 2015/08/04 13:06:35]

Yes, but technically, it would not be required to include it in the header, it
could be only forward declared, which is still very common in Chromium code,
despite the style guide now discouraging it.

For background, my line of thinking is usually as follows. When my code includes
a header X.h and depends on (the complete definition of) type T, and it so
happens that X.h includes T.h, I do not rely on this unless my code would break
(in aspects other than missing an include) if X.h ever stopped including T.h.

For example, if X is a subclass of T, or X has a public member variable of type
T by value, then I do not include T.h, because the only way to remove including
T.h in X.h implies such fundamental changes to X that it is unlikely that my
code would still be able to use X as before without being refactored itself.

[vabr (Chromium), 2015/08/04 13:37:53]

In this case, though, if the header is not included, the compilation will break
(the compiler will not know, what offset is .username_value). So this sounds
similar to the case of X inheriting T.
But as I said, no issue with adding the #include, which I did in patch set 10.

+    return false;
+
+  if (autofill_sync_state_ == DISALLOW_SYNC_CREDENTIALS) {
+    sync_credential_was_filtered_ = true;
+    return true;
+  }
+
+  const GURL last_load_url = client_->GetLastCommittedEntryURL();
+  if (autofill_sync_state_ == DISALLOW_SYNC_CREDENTIALS_FOR_REAUTH &&
+      LastLoadWasTransactionalReauthPage(last_load_url)) {
+    sync_credential_was_filtered_ = true;
+    return true;
+  }
+
+  return false;
+}
+
+// static
+SyncStoreResultFilter::AutofillForSyncCredentialsState
+SyncStoreResultFilter::SetUpAutofillSyncState() {
+  std::string group_name =
+      base::FieldTrialList::FindFullName("AutofillSyncCredential");
+
+  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(switches::kAllowAutofillSyncCredential))
+    return ALLOW_SYNC_CREDENTIALS;
+  if (command_line->HasSwitch(
+          switches::kDisallowAutofillSyncCredentialForReauth)) {
+    return DISALLOW_SYNC_CREDENTIALS_FOR_REAUTH;
+  }
+  if (command_line->HasSwitch(switches::kDisallowAutofillSyncCredential))
+    return DISALLOW_SYNC_CREDENTIALS;
+
+  if (group_name == "DisallowSyncCredentialsForReauth")
+    return DISALLOW_SYNC_CREDENTIALS_FOR_REAUTH;
+  if (group_name == "DisallowSyncCredentials")
+    return DISALLOW_SYNC_CREDENTIALS;
+
+  // Allow by default.
+  return ALLOW_SYNC_CREDENTIALS;
+}
+
+}  // namespace password_manager