Send IPC from renderer to browser on each Autocheckout page completion

Send IPC from renderer to browser on each Autocheckout page completion to guide Autocheckout manager to update step status and report Autocheckout status more accurately.

This also removes AutofillHostMsg_ClickFailed which is now covered by the new message.

BUG=260057
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=212231

[Raman Kakilate, 2013-07-15 15:33:16]

conceptually lgtm

https://codereview.chromium.org/18179015/diff/6001/components/autofill/conten...
File components/autofill/content/browser/autocheckout_manager.cc (right):

https://codereview.chromium.org/18179015/diff/6001/components/autofill/conten...
components/autofill/content/browser/autocheckout_manager.cc:254:
page_meta_data_->current_page_number) {
nit: this line is confusing to read, earlier "old_" was easier.

https://codereview.chromium.org/18179015/diff/6001/components/autofill/conten...
components/autofill/content/browser/autocheckout_manager.cc:265: page_meta_data_
= page_meta_data.Pass();
shouldn't we be resetting page_meta_data_ here?

[benquan, 2013-07-15 19:19:13]

https://codereview.chromium.org/18179015/diff/6001/components/autofill/conten...
File components/autofill/content/browser/autocheckout_manager.cc (right):

https://codereview.chromium.org/18179015/diff/6001/components/autofill/conten...
components/autofill/content/browser/autocheckout_manager.cc:254:
page_meta_data_->current_page_number) {
On 2013/07/15 15:33:16, Raman Kakilate wrote:
> nit: this line is confusing to read, earlier "old_" was easier.

put old_ back

https://codereview.chromium.org/18179015/diff/6001/components/autofill/conten...
components/autofill/content/browser/autocheckout_manager.cc:265: page_meta_data_
= page_meta_data.Pass();
On 2013/07/15 15:33:16, Raman Kakilate wrote:
> shouldn't we be resetting page_meta_data_ here?

page_meta_data_ should refer to the page_metadata for the page.

[benquan, 2013-07-16 17:07:01]

Ilya, PTAL

[Ilya Sherman, 2013-07-16 22:56:26]

LGTM % nits, thanks.

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
File components/autofill/content/browser/autocheckout_manager.cc (right):

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
components/autofill/content/browser/autocheckout_manager.cc:226:
scoped_ptr<AutocheckoutPageMetaData> old_meta_data =  page_meta_data_.Pass();
nit: One space too many after the equals sign.

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
components/autofill/content/browser/autocheckout_manager.cc:254:
old_meta_data->current_page_number) {
nit: The previous indentation for this line was correct.

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
File components/autofill/content/browser/autocheckout_manager.h (right):

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
components/autofill/content/browser/autocheckout_manager.h:53: // |status| is
the reason for the failure, it is |SUCCESS| if no errors.
Optional nit: Suggested minor wording tweaks:

"Called to signal that the renderer has completed processing a page in the
Autocheckout flow.  |status| is the reason for the failure, or |SUCCESS| if
there were no errors."

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
components/autofill/content/browser/autocheckout_manager.h:131: // Terminate the
Autocheckout flow and send Autocheckout status.
nit: Send the status where?

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
File components/autofill/content/renderer/autofill_agent.cc (right):

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
components/autofill/content/renderer/autofill_agent.cc:740: status));
nit: Looks like this fits on the previous line.

https://codereview.chromium.org/18179015/diff/10001/components/autofill/core/...
File components/autofill/core/browser/autofill_manager.cc (right):

https://codereview.chromium.org/18179015/diff/10001/components/autofill/core/...
components/autofill/core/browser/autofill_manager.cc:836: DVLOG(2) <<
"AutofillManager::OnMaybeShowAutocheckoutBubble";
nit: Do you really want to check this in, or was it just for local debugging?

https://codereview.chromium.org/18179015/diff/10001/components/autofill/core/...
File components/autofill/core/common/autofill_messages.h (right):

https://codereview.chromium.org/18179015/diff/10001/components/autofill/core/...
components/autofill/core/common/autofill_messages.h:272: // button if any.
nit: or if there was an error.

[commit-bot: I haz the power, 2013-07-17 00:06:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benquan@chromium.org/18179015/18001

[benquan, 2013-07-17 00:28:56]

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
File components/autofill/content/browser/autocheckout_manager.cc (right):

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
components/autofill/content/browser/autocheckout_manager.cc:226:
scoped_ptr<AutocheckoutPageMetaData> old_meta_data =  page_meta_data_.Pass();
On 2013/07/16 22:56:26, Ilya Sherman wrote:
> nit: One space too many after the equals sign.

Done.

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
components/autofill/content/browser/autocheckout_manager.cc:254:
old_meta_data->current_page_number) {
On 2013/07/16 22:56:26, Ilya Sherman wrote:
> nit: The previous indentation for this line was correct.

Done.

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
File components/autofill/content/browser/autocheckout_manager.h (right):

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
components/autofill/content/browser/autocheckout_manager.h:53: // |status| is
the reason for the failure, it is |SUCCESS| if no errors.
On 2013/07/16 22:56:26, Ilya Sherman wrote:
> Optional nit: Suggested minor wording tweaks:
> 
> "Called to signal that the renderer has completed processing a page in the
> Autocheckout flow.  |status| is the reason for the failure, or |SUCCESS| if
> there were no errors."

Done.

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
components/autofill/content/browser/autocheckout_manager.h:131: // Terminate the
Autocheckout flow and send Autocheckout status.
On 2013/07/16 22:56:26, Ilya Sherman wrote:
> nit: Send the status where?

Done.

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
File components/autofill/content/renderer/autofill_agent.cc (right):

https://codereview.chromium.org/18179015/diff/10001/components/autofill/conte...
components/autofill/content/renderer/autofill_agent.cc:740: status));
On 2013/07/16 22:56:26, Ilya Sherman wrote:
> nit: Looks like this fits on the previous line.

Done.

https://codereview.chromium.org/18179015/diff/10001/components/autofill/core/...
File components/autofill/core/browser/autofill_manager.cc (right):

https://codereview.chromium.org/18179015/diff/10001/components/autofill/core/...
components/autofill/core/browser/autofill_manager.cc:836: DVLOG(2) <<
"AutofillManager::OnMaybeShowAutocheckoutBubble";
On 2013/07/16 22:56:26, Ilya Sherman wrote:
> nit: Do you really want to check this in, or was it just for local debugging?

Done.

https://codereview.chromium.org/18179015/diff/10001/components/autofill/core/...
File components/autofill/core/common/autofill_messages.h (right):

https://codereview.chromium.org/18179015/diff/10001/components/autofill/core/...
components/autofill/core/common/autofill_messages.h:272: // button if any.
On 2013/07/16 22:56:26, Ilya Sherman wrote:
> nit: or if there was an error.

Done.

[commit-bot: I haz the power, 2013-07-17 03:26:12]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Cris Neckar, 2013-07-17 17:36:41]

rubber stamp lgtm for ipc message rename

[commit-bot: I haz the power, 2013-07-17 17:47:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benquan@chromium.org/18179015/18001

[commit-bot: I haz the power, 2013-07-17 19:53:58]

Sorry for I got bad news for ya.
Compile failed with a clobber build on ios_rel_device.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_rel_de...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-07-17 20:38:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benquan@chromium.org/18179015/18001

[commit-bot: I haz the power, 2013-07-18 01:44:35]

Retried try job too often on win7_aura for step(s) content_browsertests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-07-18 02:27:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benquan@chromium.org/18179015/18001

[commit-bot: I haz the power, 2013-07-18 03:52:59]

Change committed as 212231