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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 8 months ago by blundell
Modified:
1 year, 8 months 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:

Messages

Total messages: 12 (0 generated)
blundell
Hi Ilya, This is a work-in-progress; in particular, I need to fix up the tests. ...
1 year, 8 months 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() = ...
1 year, 8 months 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 ...
1 year, 8 months 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 ...
1 year, 8 months 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 ...
1 year, 8 months 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. ...
1 year, 8 months 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, ...
1 year, 8 months 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. ...
1 year, 8 months ago (2013-06-28 08:25:58 UTC) #8
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/blundell@chromium.org/17572015/58001
1 year, 8 months ago (2013-06-28 17:36:56 UTC) #9
I haz the power (commit-bot)
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 ...
1 year, 8 months ago (2013-06-28 17:36:58 UTC) #10
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/blundell@chromium.org/17572015/63001
1 year, 8 months ago (2013-06-30 16:59:30 UTC) #11
I haz the power (commit-bot)
1 year, 8 months 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 dd99357-tainted