Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
14017003:
Disabling Autocheckout on non-Views builds. (Closed)
|
Created:
7 years, 8 months ago by ahutter Modified:
7 years, 8 months ago Reviewers:
Ilya Sherman CC:
chromium-reviews, Raman Kakilate, benquan, dhollowa+watch_chromium.org, browser-components-watch_chromium.org, dbeam+watch-autofill_chromium.org, Dane Wallinga, dyu1, estade+watch_chromium.org, Albert Bodenhamer Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionDisabling Autocheckout on non-Views builds.
BUG=229710
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=195065
Patch Set 1 #
Total comments: 1
Patch Set 2 : Rebasing #Messages
Total messages: 12 (0 generated)
Ilya, PTAL. Thanks.
https://codereview.chromium.org/14017003/diff/1/components/autofill/browser/a... File components/autofill/browser/autofill_manager.cc (right): https://codereview.chromium.org/14017003/diff/1/components/autofill/browser/a... components/autofill/browser/autofill_manager.cc:886: #endif // #if defined(TOOLKIT_VIEWS) Can you move this into the AutocheckoutManager code?
On 2013/04/17 00:09:50, Ilya Sherman wrote: > https://codereview.chromium.org/14017003/diff/1/components/autofill/browser/a... > File components/autofill/browser/autofill_manager.cc (right): > > https://codereview.chromium.org/14017003/diff/1/components/autofill/browser/a... > components/autofill/browser/autofill_manager.cc:886: #endif // #if > defined(TOOLKIT_VIEWS) > Can you move this into the AutocheckoutManager code? If you'd like, it means I'd have to sprinkle MAYBE_ across all its unit tests though but I don't mind moving it if you'd prefer that.
LGTM On 2013/04/17 00:13:23, ahutter wrote: > On 2013/04/17 00:09:50, Ilya Sherman wrote: > > > https://codereview.chromium.org/14017003/diff/1/components/autofill/browser/a... > > File components/autofill/browser/autofill_manager.cc (right): > > > > > https://codereview.chromium.org/14017003/diff/1/components/autofill/browser/a... > > components/autofill/browser/autofill_manager.cc:886: #endif // #if > > defined(TOOLKIT_VIEWS) > > Can you move this into the AutocheckoutManager code? > > If you'd like, it means I'd have to sprinkle MAYBE_ across all its unit tests > though but I don't mind moving it if you'd prefer that. Ah, right, this does play more nicely with unit tests. Never mind then :)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ahutter@chromium.org/14017003/1
Failed to apply patch for components/autofill/browser/autofill_manager.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file components/autofill/browser/autofill_manager.cc
Hunk #1 FAILED at 879.
1 out of 1 hunk FAILED -- saving rejects to file
components/autofill/browser/autofill_manager.cc.rej
Patch: components/autofill/browser/autofill_manager.cc
Index: components/autofill/browser/autofill_manager.cc
diff --git a/components/autofill/browser/autofill_manager.cc
b/components/autofill/browser/autofill_manager.cc
index
ceaf77c2bcce590d5032470ea2f58568e2bec705..a6f6d014cc0a32e4a653a09dab948b2fa77e3069
100644
--- a/components/autofill/browser/autofill_manager.cc
+++ b/components/autofill/browser/autofill_manager.cc
@@ -879,7 +879,11 @@ void AutofillManager::OnLoadedServerPredictions(
page_meta_data.get(),
*metric_logger_);
+ // TODO(ahutter): Remove this once Autocheckout is implemented on other
+ // platforms. See http://crbug.com/173416.
+#if defined(TOOLKIT_VIEWS)
autocheckout_manager_.OnLoadedPageMetaData(page_meta_data.Pass());
+#endif // #if defined(TOOLKIT_VIEWS)
// If the corresponding flag is set, annotate forms with the predicted types.
SendAutofillTypePredictions(form_structures_.get());
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ahutter@chromium.org/14017003/8001
Sorry for I got bad news for ya. Compile failed with a clobber build on win7_aura. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&... Your code is likely broken or HEAD is junk. Please ensure your code is not broken then alert the build sheriffs. Look at the try server FAQ for more details.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ahutter@chromium.org/14017003/8001
Retried try job too often on win7_aura for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ahutter@chromium.org/14017003/8001
Message was sent while issue was closed.
Change committed as 195065 |
