Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2911433003:
Refactor SuppressedHTTPSFormsFetcher to use GetLoginsForSameOrganizationName. (Closed)
DescriptionRefactor SuppressedHTTPSFormsFetcher to use GetLoginsForSameOrganizationName.
This results in no change in functionality, but is done in preparation for
refactoring SuppressedHTTPSFormsFetcher to return not only suppressed HTTPS
credentials, but also two other flavors of suppressed credentials. As an
immediate benefit, this CL allows cleaning up password_manager_unittests.
BUG=720599
Review-Url: https://codereview.chromium.org/2911433003
Cr-Commit-Position: refs/heads/master@{#475325}
Committed: https://chromium.googlesource.com/chromium/src/+/74d6ed5b631582550c0dd9bf5c5f37475dc9804d
Patch Set 1 #Patch Set 2 : Polish. #Patch Set 3 : Polish. #Patch Set 4 : Rebase. #Patch Set 5 : Math. #Patch Set 6 : Rebase. #
Total comments: 26
Patch Set 7 : Fix test. #Patch Set 8 : Rebase. #Patch Set 9 : Address comments from vasilii@. #
Total comments: 8
Patch Set 10 : Addressed second rounds of comments. #Patch Set 11 : Comment + rebase. #Depends on Patchset: Messages
Total messages: 42 (33 generated)
Description was changed from ========== Refactor querying SuppressedHTTPSForms to use GetLoginsForSameOrganizationName. BUG=720599 ========== to ========== Refactor querying SuppressedHTTPSForms to use GetLoginsForSameOrganizationName. This results in no change in functionality, but allows cleaning up password_manager_unittests. BUG=720599 ==========
engedy@chromium.org changed reviewers: + vasilii@chromium.org
Description was changed from ========== Refactor querying SuppressedHTTPSForms to use GetLoginsForSameOrganizationName. This results in no change in functionality, but allows cleaning up password_manager_unittests. BUG=720599 ========== to ========== Refactor SuppressedHTTPSFormsFetcher to use GetLoginsForSameOrganizationName. This results in no change in functionality, but allows cleaning up password_manager_unittests. BUG=720599 ==========
The CQ bit was checked by engedy@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by engedy@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
@Vasilii, this is in preparation for adding the same-org-name suppressed logins histograms, PTAL.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by engedy@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by engedy@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by engedy@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by engedy@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... File components/password_manager/core/browser/form_fetcher_impl_unittest.cc (right): https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/form_fetcher_impl_unittest.cc:49: const char kTestHttpLoginURL[] = "http://example.in"; constexpr https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/form_fetcher_impl_unittest.cc:213: // the emitted |consumer_ptr| to complete the query. Update the comment https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... File components/password_manager/core/browser/password_manager_unittest.cc (right): https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/password_manager_unittest.cc:1415: .Times(AnyNumber()); Why do you duplicate it here? https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... File components/password_manager/core/browser/suppressed_https_form_fetcher.cc (right): https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher.cc:25: client_->GetPasswordStore()->GetLoginsForSameOrganizationName( Why do you think this method better suits your needs than GetLogins()? https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... File components/password_manager/core/browser/suppressed_https_form_fetcher.h (right): https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher.h:56: GURL observed_signon_realm_as_url_; const https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... File components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc (right): https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:22: const char kTestHttpURL[] = "http://one.example.com/"; Everything should be constexpr https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:90: const char* kObservedFormRealm = kTestHttpURL; Why not just using kTestHttpURL? https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:94: ProcessSuppressedHTTPSFormsConstRef(::testing::IsEmpty())); Here and below: this call is triggered by OnGetPasswordStoreResults. Thus, it should go before it. https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:112: static const PasswordFormData kNotSuppressedHTTPSCredentials[] = { The name is confusing. Not all of them are HTTPS. https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:146: ASSERT_EQ(7u, simulated_store_results.size()); What do you really want to check here? https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:150: base::MakeUnique<PasswordForm>(*simulated_store_results[1])); Are you adding it the second time?
Description was changed from ========== Refactor SuppressedHTTPSFormsFetcher to use GetLoginsForSameOrganizationName. This results in no change in functionality, but allows cleaning up password_manager_unittests. BUG=720599 ========== to ========== Refactor SuppressedHTTPSFormsFetcher to use GetLoginsForSameOrganizationName. This results in no change in functionality, but is done in preparation for refactoring SuppressedHTTPSFormsFetcher to return not only suppressed HTTPS credentials, but also two other flavors of suppressed credentials. As an immediate benefit, this CL allows cleaning up password_manager_unittests. BUG=720599 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Thanks, and sorry for the many nits. PTAL. https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... File components/password_manager/core/browser/form_fetcher_impl_unittest.cc (right): https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/form_fetcher_impl_unittest.cc:49: const char kTestHttpLoginURL[] = "http://example.in"; On 2017/05/26 12:18:00, vasilii wrote: > constexpr Done in 7 places. https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/form_fetcher_impl_unittest.cc:213: // the emitted |consumer_ptr| to complete the query. On 2017/05/26 12:18:00, vasilii wrote: > Update the comment Good catch, done. https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... File components/password_manager/core/browser/password_manager_unittest.cc (right): https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/password_manager_unittest.cc:1415: .Times(AnyNumber()); On 2017/05/26 12:18:00, vasilii wrote: > Why do you duplicate it here? Unfortunately, at the end of the first iteration, the ::testing::Mock::VerifyAndClearExpectations calls will wipe it for the second iteration. I moved it to after the problematic VerifyAndClearExpectations() line, though. https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... File components/password_manager/core/browser/suppressed_https_form_fetcher.cc (right): https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher.cc:25: client_->GetPasswordStore()->GetLoginsForSameOrganizationName( On 2017/05/26 12:18:00, vasilii wrote: > Why do you think this method better suits your needs than GetLogins()? You are right: for now, it doesn't.* In a follow-up CL, however, I'm refactoring this class to return not only suppressed HTTPS credentials, but 2 other flavors too. I wanted to do this refactoring step in an early CL, so as to reduce your review workload (by what will be dependent code I am adding for the other 2 flavors too). I have mentioned this in the CL description. *: There is a tiny advantage that it already makes the tests cleaner as we can distinguish these PasswordStore requests from the primary look-ups. https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... File components/password_manager/core/browser/suppressed_https_form_fetcher.h (right): https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher.h:56: GURL observed_signon_realm_as_url_; On 2017/05/26 12:18:00, vasilii wrote: > const Done. https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... File components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc (right): https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:22: const char kTestHttpURL[] = "http://one.example.com/"; On 2017/05/26 12:18:00, vasilii wrote: > Everything should be constexpr Done. https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:90: const char* kObservedFormRealm = kTestHttpURL; On 2017/05/26 12:18:00, vasilii wrote: > Why not just using kTestHttpURL? Hmm, why not, indeed. Done. https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:94: ProcessSuppressedHTTPSFormsConstRef(::testing::IsEmpty())); On 2017/05/26 12:18:00, vasilii wrote: > Here and below: this call is triggered by OnGetPasswordStoreResults. Thus, it > should go before it. Done. https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:112: static const PasswordFormData kNotSuppressedHTTPSCredentials[] = { On 2017/05/26 12:18:00, vasilii wrote: > The name is confusing. Not all of them are HTTPS. Good point! Rename to kOtherCredentials. https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:146: ASSERT_EQ(7u, simulated_store_results.size()); On 2017/05/26 12:18:00, vasilii wrote: > What do you really want to check here? What I *have been able* to check with it is that I cannot do basic mental arithmetic. But otherwise, I agree that it's probably not good for anything else. Removed. :-) https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:150: base::MakeUnique<PasswordForm>(*simulated_store_results[1])); On 2017/05/26 12:18:00, vasilii wrote: > Are you adding it the second time? Yep, that was a bug, removed in the meantime.
https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... File components/password_manager/core/browser/suppressed_https_form_fetcher.cc (right): https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher.cc:25: client_->GetPasswordStore()->GetLoginsForSameOrganizationName( On 2017/05/26 13:05:21, engedy wrote: > On 2017/05/26 12:18:00, vasilii wrote: > > Why do you think this method better suits your needs than GetLogins()? > > You are right: for now, it doesn't.* In a follow-up CL, however, I'm refactoring > this class to return not only suppressed HTTPS credentials, but 2 other flavors > too. I wanted to do this refactoring step in an early CL, so as to reduce your > review workload (by what will be dependent code I am adding for the other 2 > flavors too). I have mentioned this in the CL description. > > *: There is a tiny advantage that it already makes the tests cleaner as we can > distinguish these PasswordStore requests from the primary look-ups. Acknowledged. https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... File components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc (right): https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:146: ASSERT_EQ(7u, simulated_store_results.size()); On 2017/05/26 13:05:21, engedy wrote: > On 2017/05/26 12:18:00, vasilii wrote: > > What do you really want to check here? > > What I *have been able* to check with it is that I cannot do basic mental > arithmetic. But otherwise, I agree that it's probably not good for anything > else. Removed. :-) Probably, not a task for trybots ;-) https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:150: base::MakeUnique<PasswordForm>(*simulated_store_results[1])); On 2017/05/26 13:05:21, engedy wrote: > On 2017/05/26 12:18:00, vasilii wrote: > > Are you adding it the second time? > > Yep, that was a bug, removed in the meantime. Strange that the test passed.. https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... File components/password_manager/core/browser/form_fetcher_impl.cc (right): https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... components/password_manager/core/browser/form_fetcher_impl.cc:148: base::MakeUnique<SuppressedHTTPSFormFetcher>(form_digest_.signon_realm, Unrelated to the CL but why do we request the credentials so late? https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... File components/password_manager/core/browser/password_manager_unittest.cc (right): https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... components/password_manager/core/browser/password_manager_unittest.cc:1441: .Times(AnyNumber()); Can you explain what triggers this? 'Update' shouldn't cause the call. Then we should place the expectation before the trigger. Also why would it be more than once? https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... File components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc (right): https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:101: static const PasswordFormData kSuppressedHTTPSCredentials[] = { here and below constexpr
https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... File components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc (right): https://codereview.chromium.org/2911433003/diff/100001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:150: base::MakeUnique<PasswordForm>(*simulated_store_results[1])); On 2017/05/26 14:02:28, vasilii wrote: > On 2017/05/26 13:05:21, engedy wrote: > > On 2017/05/26 12:18:00, vasilii wrote: > > > Are you adding it the second time? > > > > Yep, that was a bug, removed in the meantime. > > Strange that the test passed.. Don't worry, it failed, I just fixed it in the meantime. https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... File components/password_manager/core/browser/form_fetcher_impl.cc (right): https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... components/password_manager/core/browser/form_fetcher_impl.cc:148: base::MakeUnique<SuppressedHTTPSFormFetcher>(form_digest_.signon_realm, On 2017/05/26 14:02:28, vasilii wrote: > Unrelated to the CL but why do we request the credentials so late? From the looks of it, we still instantiate many FormFetchers per page, so this was my poor man's solution to de-prioritize the suppressed requests, as this data is only needed at PasswordFormManager destruction time. My thinking was that by the time the first result comes back, we would have a bunch of GetLogins() calls already queued on the database thread, so the sequence of calls would be this: GetLogins (high prio) GetLogins (high prio) GetLogins (high prio) GetLoginsForSameOrganizationName (low prio) GetLoginsForSameOrganizationName (low prio) GetLoginsForSameOrganizationName (low prio) Instead of what would happen if we triggered the calls earlier, and potentially delaying filled logins: GetLogins (high prio) GetLoginsForSameOrganizationName (low prio) GetLogins (high prio) GetLoginsForSameOrganizationName (low prio) GetLogins (high prio) GetLoginsForSameOrganizationName (low prio) https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... File components/password_manager/core/browser/password_manager_unittest.cc (right): https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... components/password_manager/core/browser/password_manager_unittest.cc:1441: .Times(AnyNumber()); On 2017/05/26 14:02:28, vasilii wrote: > Can you explain what triggers this? 'Update' shouldn't cause the call. Then we > should place the expectation before the trigger. > Also why would it be more than once? So, hmm, that's "after VerifyAndClear", not "before Update". :-) But how about this? https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... File components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc (right): https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... components/password_manager/core/browser/suppressed_https_form_fetcher_unittest.cc:101: static const PasswordFormData kSuppressedHTTPSCredentials[] = { On 2017/05/26 14:02:28, vasilii wrote: > here and below constexpr Done.
The CQ bit was checked by engedy@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... File components/password_manager/core/browser/form_fetcher_impl.cc (right): https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... components/password_manager/core/browser/form_fetcher_impl.cc:148: base::MakeUnique<SuppressedHTTPSFormFetcher>(form_digest_.signon_realm, On 2017/05/26 14:14:41, engedy wrote: > On 2017/05/26 14:02:28, vasilii wrote: > > Unrelated to the CL but why do we request the credentials so late? > > From the looks of it, we still instantiate many FormFetchers per page, so this > was my poor man's solution to de-prioritize the suppressed requests, as this > data is only needed at PasswordFormManager destruction time. > > My thinking was that by the time the first result comes back, we would have a > bunch of GetLogins() calls already queued on the database thread, so the > sequence of calls would be this: > > GetLogins (high prio) > GetLogins (high prio) > GetLogins (high prio) > GetLoginsForSameOrganizationName (low prio) > GetLoginsForSameOrganizationName (low prio) > GetLoginsForSameOrganizationName (low prio) > > Instead of what would happen if we triggered the calls earlier, and potentially > delaying filled logins: > GetLogins (high prio) > GetLoginsForSameOrganizationName (low prio) > GetLogins (high prio) > GetLoginsForSameOrganizationName (low prio) > GetLogins (high prio) > GetLoginsForSameOrganizationName (low prio) Agree. We should have one FormFetcher per page hopefully in M61
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by engedy@chromium.org to run a CQ dry run
https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... File components/password_manager/core/browser/form_fetcher_impl.cc (right): https://codereview.chromium.org/2911433003/diff/160001/components/password_ma... components/password_manager/core/browser/form_fetcher_impl.cc:148: base::MakeUnique<SuppressedHTTPSFormFetcher>(form_digest_.signon_realm, On 2017/05/26 14:33:50, vasilii wrote: > On 2017/05/26 14:14:41, engedy wrote: > > On 2017/05/26 14:02:28, vasilii wrote: > > > Unrelated to the CL but why do we request the credentials so late? > > > > From the looks of it, we still instantiate many FormFetchers per page, so this > > was my poor man's solution to de-prioritize the suppressed requests, as this > > data is only needed at PasswordFormManager destruction time. > > > > My thinking was that by the time the first result comes back, we would have a > > bunch of GetLogins() calls already queued on the database thread, so the > > sequence of calls would be this: > > > > GetLogins (high prio) > > GetLogins (high prio) > > GetLogins (high prio) > > GetLoginsForSameOrganizationName (low prio) > > GetLoginsForSameOrganizationName (low prio) > > GetLoginsForSameOrganizationName (low prio) > > > > Instead of what would happen if we triggered the calls earlier, and > potentially > > delaying filled logins: > > GetLogins (high prio) > > GetLoginsForSameOrganizationName (low prio) > > GetLogins (high prio) > > GetLoginsForSameOrganizationName (low prio) > > GetLogins (high prio) > > GetLoginsForSameOrganizationName (low prio) > > Agree. We should have one FormFetcher per page hopefully in M61 Really cool! In any case, I added a comment here to explain the rationale.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by engedy@chromium.org
The CQ bit was checked by engedy@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from vasilii@chromium.org Link to the patchset: https://codereview.chromium.org/2911433003/#ps200001 (title: "Comment + rebase.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 200001, "attempt_start_ts": 1496050779284220,
"parent_rev": "7a272639890aee580c26521eae1d958806f9944e", "commit_rev":
"74d6ed5b631582550c0dd9bf5c5f37475dc9804d"}
Message was sent while issue was closed.
Description was changed from ========== Refactor SuppressedHTTPSFormsFetcher to use GetLoginsForSameOrganizationName. This results in no change in functionality, but is done in preparation for refactoring SuppressedHTTPSFormsFetcher to return not only suppressed HTTPS credentials, but also two other flavors of suppressed credentials. As an immediate benefit, this CL allows cleaning up password_manager_unittests. BUG=720599 ========== to ========== Refactor SuppressedHTTPSFormsFetcher to use GetLoginsForSameOrganizationName. This results in no change in functionality, but is done in preparation for refactoring SuppressedHTTPSFormsFetcher to return not only suppressed HTTPS credentials, but also two other flavors of suppressed credentials. As an immediate benefit, this CL allows cleaning up password_manager_unittests. BUG=720599 Review-Url: https://codereview.chromium.org/2911433003 Cr-Commit-Position: refs/heads/master@{#475325} Committed: https://chromium.googlesource.com/chromium/src/+/74d6ed5b631582550c0dd9bf5c5f... ==========
Message was sent while issue was closed.
Committed patchset #11 (id:200001) as https://chromium.googlesource.com/chromium/src/+/74d6ed5b631582550c0dd9bf5c5f... |
