Chromium Code Reviews
Help | Chromium Project | Sign in
(146)

Issue 17572015: Begin abstracting sending of IPC from autofill core code. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 1 month ago by blundell
Modified:
2 years, 1 month ago
CC:
chromium-reviews, Raman Kakilate, benquan, jam, ahutter, browser-components-watch_chromium.org, joi+watch-content_chromium.org, darin-cc_chromium.org, Dane Wallinga, dyu1, Albert Bodenhamer, estade+watch_chromium.org, Ilya Sherman (Away 7.18-8.03), rouslan+autofillwatch_chromium.org, selim, benm (inactive)
Visibility:
Public.

Description

Begin abstracting sending of IPC from autofill core code. This CL introduces the pattern via which sending of IPC from autofill core code will instead be abstracted through AutofillDriver. Concretely, it abstracts the sending of the AutofillMsg_FormDataFilled message. BUG=247015 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=209375

Patch Set 1 #

Total comments: 4

Patch Set 2 : Response to review, add tests #

Total comments: 14

Patch Set 3 : Response to review #

Total comments: 2

Patch Set 4 : Response to review #

Patch Set 5 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+463 lines, -405 lines) Patch
M components/autofill/content/browser/autofill_driver_impl.h View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M components/autofill/content/browser/autofill_driver_impl.cc View 1 2 3 4 2 chunks +14 lines, -0 lines 0 comments Download
M components/autofill/content/browser/autofill_driver_impl_unittest.cc View 1 2 3 3 chunks +40 lines, -0 lines 0 comments Download
M components/autofill/core/browser/autofill_common_test.h View 1 2 chunks +6 lines, -0 lines 0 comments Download
M components/autofill/core/browser/autofill_common_test.cc View 1 2 2 chunks +33 lines, -0 lines 0 comments Download
M components/autofill/core/browser/autofill_driver.h View 1 2 2 chunks +13 lines, -2 lines 0 comments Download
M components/autofill/core/browser/autofill_manager.cc View 1 3 chunks +4 lines, -6 lines 0 comments Download
M components/autofill/core/browser/autofill_manager_unittest.cc View 1 2 3 4 60 chunks +339 lines, -397 lines 0 comments Download
M components/autofill/core/browser/test_autofill_driver.h View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M components/autofill/core/browser/test_autofill_driver.cc View 1 2 1 chunk +8 lines, -0 lines 0 comments Download
Commit: CQ not working?

Messages

Total messages: 12 (0 generated)
blundell
Hi Ilya, This is a work-in-progress; in particular, I need to fix up the tests. ...
2 years, 1 month ago (2013-06-24 20:25:13 UTC) #1
Ilya Sherman (Away 7.18-8.03)
General approach LG, thanks :) https://codereview.chromium.org/17572015/diff/1/components/autofill/core/browser/autofill_driver.h File components/autofill/core/browser/autofill_driver.h (right): https://codereview.chromium.org/17572015/diff/1/components/autofill/core/browser/autofill_driver.h#newcode28 components/autofill/core/browser/autofill_driver.h:28: virtual bool CanSendToRenderer() = ...
2 years, 1 month ago (2013-06-25 01:22:33 UTC) #2
blundell
Hi Ilya, Thanks! This is now complete with the exception of autofill_manager_unittest.cc. There are several ...
2 years, 1 month ago (2013-06-25 16:11:03 UTC) #3
blundell
On 2013/06/25 16:11:03, blundell wrote: > Hi Ilya, > > Thanks! This is now complete ...
2 years, 1 month ago (2013-06-26 19:05:00 UTC) #4
Ilya Sherman (Away 7.18-8.03)
https://codereview.chromium.org/17572015/diff/8002/components/autofill/content/browser/autofill_driver_impl.h File components/autofill/content/browser/autofill_driver_impl.h (right): https://codereview.chromium.org/17572015/diff/8002/components/autofill/content/browser/autofill_driver_impl.h#newcode48 components/autofill/content/browser/autofill_driver_impl.h:48: OVERRIDE; nit: IMO, it would be slightly cleaner to ...
2 years, 1 month ago (2013-06-26 22:54:58 UTC) #5
blundell
Thanks! Fixed up all tests as well as addressing comments. The autofill_manager_unittest churn is unfortunate. ...
2 years, 1 month ago (2013-06-27 21:58:26 UTC) #6
Ilya Sherman (Away 7.18-8.03)
LGTM, thanks. https://codereview.chromium.org/17572015/diff/42001/components/autofill/content/browser/autofill_driver_impl_unittest.cc File components/autofill/content/browser/autofill_driver_impl_unittest.cc (right): https://codereview.chromium.org/17572015/diff/42001/components/autofill/content/browser/autofill_driver_impl_unittest.cc#newcode82 components/autofill/content/browser/autofill_driver_impl_unittest.cc:82: // IPC messages. If none is present, ...
2 years, 1 month ago (2013-06-27 23:13:02 UTC) #7
blundell
https://codereview.chromium.org/17572015/diff/42001/components/autofill/content/browser/autofill_driver_impl_unittest.cc File components/autofill/content/browser/autofill_driver_impl_unittest.cc (right): https://codereview.chromium.org/17572015/diff/42001/components/autofill/content/browser/autofill_driver_impl_unittest.cc#newcode82 components/autofill/content/browser/autofill_driver_impl_unittest.cc:82: // IPC messages. If none is present, returns false. ...
2 years, 1 month ago (2013-06-28 08:25:58 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/blundell@chromium.org/17572015/58001
2 years, 1 month ago (2013-06-28 17:36:56 UTC) #9
commit-bot: I haz the power
Failed to apply patch for components/autofill/content/browser/autofill_driver_impl.cc: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
2 years, 1 month ago (2013-06-28 17:36:58 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/blundell@chromium.org/17572015/63001
2 years, 1 month ago (2013-06-30 16:59:30 UTC) #11
commit-bot: I haz the power
2 years, 1 month ago (2013-07-01 07:18:00 UTC) #12
Message was sent while issue was closed.
Change committed as 209375
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 5fa3ca5