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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 months, 3 weeks ago by blundell (OOO until April 28)
Modified:
9 months, 2 weeks ago
Reviewers:
Ilya Sherman
CC:
chromium-reviews_chromium.org, 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
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) Lint Patch
M components/autofill/content/browser/autofill_driver_impl.h View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments 0 errors Download
M components/autofill/content/browser/autofill_driver_impl.cc View 1 2 3 4 2 chunks +14 lines, -0 lines 0 comments 0 errors Download
M components/autofill/content/browser/autofill_driver_impl_unittest.cc View 1 2 3 3 chunks +40 lines, -0 lines 0 comments 0 errors Download
M components/autofill/core/browser/autofill_common_test.h View 1 2 chunks +6 lines, -0 lines 0 comments 0 errors Download
M components/autofill/core/browser/autofill_common_test.cc View 1 2 2 chunks +33 lines, -0 lines 0 comments 0 errors Download
M components/autofill/core/browser/autofill_driver.h View 1 2 2 chunks +13 lines, -2 lines 0 comments 0 errors Download
M components/autofill/core/browser/autofill_manager.cc View 1 3 chunks +4 lines, -6 lines 0 comments 0 errors Download
M components/autofill/core/browser/autofill_manager_unittest.cc View 1 2 3 4 60 chunks +339 lines, -397 lines 0 comments 0 errors Download
M components/autofill/core/browser/test_autofill_driver.h View 1 2 1 chunk +3 lines, -0 lines 0 comments 0 errors Download
M components/autofill/core/browser/test_autofill_driver.cc View 1 2 1 chunk +8 lines, -0 lines 0 comments 0 errors Download
Commit:

Messages

Total messages: 12
blundell (OOO until April 28)
Hi Ilya, This is a work-in-progress; in particular, I need to fix up the tests. ...
9 months, 3 weeks ago #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() = ...
9 months, 3 weeks ago #2
blundell (OOO until April 28)
Hi Ilya, Thanks! This is now complete with the exception of autofill_manager_unittest.cc. There are several ...
9 months, 3 weeks ago #3
blundell (OOO until April 28)
On 2013/06/25 16:11:03, blundell wrote: > Hi Ilya, > > Thanks! This is now complete ...
9 months, 3 weeks ago #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 ...
9 months, 3 weeks ago #5
blundell (OOO until April 28)
Thanks! Fixed up all tests as well as addressing comments. The autofill_manager_unittest churn is unfortunate. ...
9 months, 3 weeks ago #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, ...
9 months, 3 weeks ago #7
blundell (OOO until April 28)
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. ...
9 months, 3 weeks ago #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
9 months, 3 weeks ago #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 ...
9 months, 3 weeks ago #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
9 months, 3 weeks ago #11
I haz the power (commit-bot)
9 months, 2 weeks ago #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 1280:2d3e6564b7b6