Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(213)

Issue 25405003: [rAC, OSX] saveInChrome status now persistent. (Closed)

Created:
7 years, 2 months ago by groby-ooo-7-16
Modified:
7 years, 2 months ago
Reviewers:
Avi (use Gerrit)
CC:
chromium-reviews, benquan, Dane Wallinga, dyu1, estade+watch_chromium.org, Albert Bodenhamer, Ilya Sherman, rouslan+autofillwatch_chromium.org
Visibility:
Public.

Description

[rAC, OSX] saveInChrome status now persistent. The AutofillDialogController already persisted the state of the "Save in Chrome" checkbox. This CL ensures that the OSX dialog reflects that persisted state and is properly updated when the state changes. NOTRY=true BUG=282101 R=avi@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=226376

Patch Set 1 #

Total comments: 2

Patch Set 2 : Review fix. #

Total comments: 2

Patch Set 3 : One more space. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -2 lines) Patch
M chrome/browser/ui/cocoa/autofill/autofill_dialog_cocoa.mm View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/autofill/autofill_main_container.h View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/autofill/autofill_main_container.mm View 1 2 3 chunks +7 lines, -2 lines 0 comments Download

Messages

Total messages: 15 (0 generated)
groby-ooo-7-16
7 years, 2 months ago (2013-10-01 00:18:06 UTC) #1
Avi (use Gerrit)
lgtm mod formatting https://codereview.chromium.org/25405003/diff/1/chrome/browser/ui/cocoa/autofill/autofill_main_container.mm File chrome/browser/ui/cocoa/autofill/autofill_main_container.mm (right): https://codereview.chromium.org/25405003/diff/1/chrome/browser/ui/cocoa/autofill/autofill_main_container.mm#newcode59 chrome/browser/ui/cocoa/autofill/autofill_main_container.mm:59: (delegate_->ShouldSaveInChrome()?NSOnState:NSOffState)]; Formatting off. Indent 4, spaces ...
7 years, 2 months ago (2013-10-01 00:24:01 UTC) #2
groby-ooo-7-16
https://codereview.chromium.org/25405003/diff/1/chrome/browser/ui/cocoa/autofill/autofill_main_container.mm File chrome/browser/ui/cocoa/autofill/autofill_main_container.mm (right): https://codereview.chromium.org/25405003/diff/1/chrome/browser/ui/cocoa/autofill/autofill_main_container.mm#newcode59 chrome/browser/ui/cocoa/autofill/autofill_main_container.mm:59: (delegate_->ShouldSaveInChrome()?NSOnState:NSOffState)]; On 2013/10/01 00:24:01, Avi wrote: > Formatting off. ...
7 years, 2 months ago (2013-10-01 01:35:39 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/groby@chromium.org/25405003/16001
7 years, 2 months ago (2013-10-01 01:36:44 UTC) #4
Avi (use Gerrit)
not lgtm https://codereview.chromium.org/25405003/diff/16001/chrome/browser/ui/cocoa/autofill/autofill_main_container.mm File chrome/browser/ui/cocoa/autofill/autofill_main_container.mm (right): https://codereview.chromium.org/25405003/diff/16001/chrome/browser/ui/cocoa/autofill/autofill_main_container.mm#newcode59 chrome/browser/ui/cocoa/autofill/autofill_main_container.mm:59: (delegate_->ShouldSaveInChrome() ? NSOnState : NSOffState)]; Indentation is ...
7 years, 2 months ago (2013-10-01 02:06:08 UTC) #5
groby-ooo-7-16
Apologies, must've gone cross-eyed. https://chromiumcodereview.appspot.com/25405003/diff/16001/chrome/browser/ui/cocoa/autofill/autofill_main_container.mm File chrome/browser/ui/cocoa/autofill/autofill_main_container.mm (right): https://chromiumcodereview.appspot.com/25405003/diff/16001/chrome/browser/ui/cocoa/autofill/autofill_main_container.mm#newcode59 chrome/browser/ui/cocoa/autofill/autofill_main_container.mm:59: (delegate_->ShouldSaveInChrome() ? NSOnState : NSOffState)]; ...
7 years, 2 months ago (2013-10-01 02:12:02 UTC) #6
commit-bot: I haz the power
Failed to trigger a try job on win_x64_rel HTTP Error 400: Bad Request
7 years, 2 months ago (2013-10-01 02:13:25 UTC) #7
Avi (use Gerrit)
OK, cool. lgtm
7 years, 2 months ago (2013-10-01 02:15:23 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/groby@chromium.org/25405003/22001
7 years, 2 months ago (2013-10-01 21:47:15 UTC) #9
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 2 months ago (2013-10-01 22:47:54 UTC) #10
groby-ooo-7-16
NOTRY=true because the CQ hates me
7 years, 2 months ago (2013-10-02 01:10:26 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/groby@chromium.org/25405003/22001
7 years, 2 months ago (2013-10-02 01:12:07 UTC) #12
commit-bot: I haz the power
Change committed as 226376
7 years, 2 months ago (2013-10-02 01:12:47 UTC) #13
piman
On 2013/10/02 01:10:26, groby wrote: > NOTRY=true because the CQ hates me The mac failures ...
7 years, 2 months ago (2013-10-02 02:46:07 UTC) #14
groby-ooo-7-16
7 years, 2 months ago (2013-10-02 22:44:14 UTC) #15
Ouch. My apologies for committing this hastily. Investigating why it passed
locally.

(This CL is cursed)


On Tue, Oct 1, 2013 at 7:46 PM, <piman@chromium.org> wrote:

> On 2013/10/02 01:10:26, groby wrote:
>
>> NOTRY=true because the CQ hates me
>>
>
> The mac failures were real though, and this broke the build
> http://build.chromium.org/p/**chromium.mac/builders/Mac10.6%**
> 20Tests%20%282%29/builds/**42927/steps/unit_tests/logs/**
>
SaveDetailsLocallyDefaultsToTr**ue<http://build.chromium.org/p/chromium.mac/builders/Mac10.6%20Tests%20%282%29/builds/42927/steps/unit_tests/logs/SaveDetailsLocallyDefaultsToTrue>
>
>
https://codereview.chromium.**org/25405003/<https://codereview.chromium.org/2...
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

Powered by Google App Engine
This is Rietveld 408576698