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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years ago by blundell
Modified:
2 years ago
Reviewers:
Ilya Sherman
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, 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 ago (2013-06-24 20:25:13 UTC) #1
Ilya Sherman
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 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 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 ago (2013-06-26 19:05:00 UTC) #4
Ilya Sherman
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 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 ago (2013-06-27 21:58:26 UTC) #6
Ilya Sherman
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 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 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 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 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 ago (2013-06-30 16:59:30 UTC) #11
commit-bot: I haz the power
2 years 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 1f9106d