Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
15487004:
Autocheckout: parse multiple clicks setting in autofill response. (Closed)
|
Created:
7 years, 7 months ago by benquan Modified:
7 years, 6 months ago CC:
chromium-reviews, Raman Kakilate, ahutter, browser-components-watch_chromium.org, dbeam+watch-autofill_chromium.org, Dane Wallinga, dyu1, Albert Bodenhamer, estade+watch_chromium.org, Ilya Sherman Base URL:
https://chromium.googlesource.com/chromium/src.git@master Visibility:
Public. |
DescriptionAutocheckout: parse multiple clicks setting in autofill response.
BUG=242394
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=203854
Patch Set 1 #
Total comments: 3
Patch Set 2 : remove wait_time_ms #
Total comments: 20
Patch Set 3 : address Ilya's comments. #
Total comments: 2
Patch Set 4 : remove return value from ParseElementDescriptor() #
Total comments: 6
Patch Set 5 : Ily's comments. #
Total comments: 2
Patch Set 6 : Ilya's comments #
Messages
Total messages: 20 (0 generated)
https://codereview.chromium.org/15487004/diff/1/components/autofill/browser/a... File components/autofill/browser/autocheckout_page_meta_data.h (right): https://codereview.chromium.org/15487004/diff/1/components/autofill/browser/a... components/autofill/browser/autocheckout_page_meta_data.h:22: int wait_time_ms; How are the times chosen? What guarantees that the merchant's script will complete in any specific amount of time? The time required for the script to run seems like it would depend a lot on the user's hardware and the load on that hardware...
https://codereview.chromium.org/15487004/diff/1/components/autofill/browser/a... File components/autofill/browser/autocheckout_page_meta_data.h (right): https://codereview.chromium.org/15487004/diff/1/components/autofill/browser/a... components/autofill/browser/autocheckout_page_meta_data.h:22: int wait_time_ms; On 2013/05/21 04:25:04, Ilya Sherman wrote: > How are the times chosen? What guarantees that the merchant's script will > complete in any specific amount of time? The time required for the script to > run seems like it would depend a lot on the user's hardware and the load on that > hardware... You are absolutely correct, the time required to complete merchant's script depends on many factors, and there is no easy and clean way to detect that. So we have to set it conservatively so that it covers majority of the users. But, in the majority of the sites, we do not need to set a wait_time at all. If we do need it for a few sites, I would expect merchant's script just simply show/hide or enable/disable some DOM objects that are already loaded in the page, it should be relatively quick. If the merchant use AJAX to bring in new forms, we treats them as a new Autocheckout page, and query autofill server for autofill-info for the new form, and thus a new AutocheckoutPageMetaData will be retrieved, so wait_time_ms is not required.
https://codereview.chromium.org/15487004/diff/1/components/autofill/browser/a... File components/autofill/browser/autocheckout_page_meta_data.h (right): https://codereview.chromium.org/15487004/diff/1/components/autofill/browser/a... components/autofill/browser/autocheckout_page_meta_data.h:22: int wait_time_ms; On 2013/05/21 17:17:08, benquan wrote: > On 2013/05/21 04:25:04, Ilya Sherman wrote: > > How are the times chosen? What guarantees that the merchant's script will > > complete in any specific amount of time? The time required for the script to > > run seems like it would depend a lot on the user's hardware and the load on > that > > hardware... > > You are absolutely correct, the time required to complete merchant's script > depends on many factors, and there is no easy and clean way to detect that. So > we have to set it conservatively so that it covers majority of the users. > > But, in the majority of the sites, we do not need to set a wait_time at all. By this, you mean that for the majority of the sites, this entire CL is unneeded -- right? > If we do need it for a few sites, I would expect merchant's script just simply > show/hide or enable/disable some DOM objects that are already loaded in the > page, it should be relatively quick. If the merchant use AJAX to bring in new > forms, we treats them as a new Autocheckout page, and query autofill server for > autofill-info for the new form, and thus a new AutocheckoutPageMetaData will be > retrieved, so wait_time_ms is not required. Let me reiterate: Relying on hardcoded timeouts is flaky (because even a conservative setting will not cover all users) and slow (because a conservative setting implies a lot of wasted time for most users). We should block on explicit DOM events instead, or find some other deterministic way to identify when a step in the purchase flow has completed.
On 2013/05/21 22:48:40, Ilya Sherman wrote: > https://codereview.chromium.org/15487004/diff/1/components/autofill/browser/a... > File components/autofill/browser/autocheckout_page_meta_data.h (right): > > https://codereview.chromium.org/15487004/diff/1/components/autofill/browser/a... > components/autofill/browser/autocheckout_page_meta_data.h:22: int wait_time_ms; > On 2013/05/21 17:17:08, benquan wrote: > > On 2013/05/21 04:25:04, Ilya Sherman wrote: > > > How are the times chosen? What guarantees that the merchant's script will > > > complete in any specific amount of time? The time required for the script > to > > > run seems like it would depend a lot on the user's hardware and the load on > > that > > > hardware... > > > > You are absolutely correct, the time required to complete merchant's script > > depends on many factors, and there is no easy and clean way to detect that. So > > we have to set it conservatively so that it covers majority of the users. > > > > But, in the majority of the sites, we do not need to set a wait_time at all. > > By this, you mean that for the majority of the sites, this entire CL is unneeded > -- right? I meant some sites need multiple clicks, and majority of such sites do not need waiting between clicks (we can continually click them). This CL is needed, only the wait_time is not needed. > > > If we do need it for a few sites, I would expect merchant's script just simply > > show/hide or enable/disable some DOM objects that are already loaded in the > > page, it should be relatively quick. If the merchant use AJAX to bring in new > > forms, we treats them as a new Autocheckout page, and query autofill server > for > > autofill-info for the new form, and thus a new AutocheckoutPageMetaData will > be > > retrieved, so wait_time_ms is not required. > > Let me reiterate: Relying on hardcoded timeouts is flaky (because even a > conservative setting will not cover all users) and slow (because a conservative > setting implies a lot of wasted time for most users). We should block on > explicit DOM events instead, or find some other deterministic way to identify > when a step in the purchase flow has completed. First of all, this multi-click is for clicking multiple web elements that are already in the page, which is within one step in the checkout flow, no expensive/slow client-server interaction or AJAX are involved after click. AJAX are considered separate steps, and next steps are triggered by receiving of AJAX response, not by wait_time. Anyways, we have not yet found a site that really need waiting between clicks yet, I think we can drop it for now.
Ok. Let's drop the waiting for now, then. I'll be OOO 'til next Wednesday, so you might want to pass this review to Evan once you update the CL. Otherwise, I can look at it again when I get back. Thanks.
PTAL
Adding Evan as a reviewer, as I'll be OOO until next Wednesday. https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... File components/autofill/browser/autocheckout_page_meta_data.h (right): https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autocheckout_page_meta_data.h:42: // Elements have to be clicked in order, and may wait between clicks. nit: No waiting. https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autocheckout_page_meta_data.h:43: std::vector<WebElementDescriptor> click_elements_before_formfill; nit: "formfill" -> "form_fill" or something like that https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autocheckout_page_meta_data.h:46: // page_advance_button. Why not just make the page_advance_button always be the last element that needs to be clicked after filling the form? https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autocheckout_page_meta_data.h:47: // Elements have to be clicked in order, and may wait between clicks. nit: No waiting. https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autocheckout_page_meta_data.h:48: std::vector<WebElementDescriptor> click_elements_after_formfill; nit: "formfill" -> "form_fill" or something like that https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... File components/autofill/browser/autofill_xml_parser.cc (right): https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autofill_xml_parser.cc:141: current_click_element_ = &click_elements->back(); This doesn't look like it's actually parsed anything... https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autofill_xml_parser.cc:145: buzz::XmlParseContext* context, nit: Can this be passed by const-reference? https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... File components/autofill/browser/autofill_xml_parser.h (right): https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autofill_xml_parser.h:103: const char** attrs, Please document that |attrs| will be modified by calling this method.
https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... File components/autofill/browser/autocheckout_page_meta_data.h (right): https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autocheckout_page_meta_data.h:42: // Elements have to be clicked in order, and may wait between clicks. On 2013/05/23 09:04:42, Ilya Sherman wrote: > nit: No waiting. Done. https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autocheckout_page_meta_data.h:43: std::vector<WebElementDescriptor> click_elements_before_formfill; On 2013/05/23 09:04:42, Ilya Sherman wrote: > nit: "formfill" -> "form_fill" or something like that Done. https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autocheckout_page_meta_data.h:46: // page_advance_button. On 2013/05/23 09:04:42, Ilya Sherman wrote: > Why not just make the page_advance_button always be the last element that needs > to be clicked after filling the form? page_advance_button is already in the proto and our fieldmapping files. We could deprecate it and use this instead later. https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autocheckout_page_meta_data.h:47: // Elements have to be clicked in order, and may wait between clicks. On 2013/05/23 09:04:42, Ilya Sherman wrote: > nit: No waiting. Done. https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autocheckout_page_meta_data.h:48: std::vector<WebElementDescriptor> click_elements_after_formfill; On 2013/05/23 09:04:42, Ilya Sherman wrote: > nit: "formfill" -> "form_fill" or something like that Done. https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... File components/autofill/browser/autofill_xml_parser.cc (right): https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autofill_xml_parser.cc:141: current_click_element_ = &click_elements->back(); On 2013/05/23 09:04:42, Ilya Sherman wrote: > This doesn't look like it's actually parsed anything... renamed https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autofill_xml_parser.cc:145: buzz::XmlParseContext* context, On 2013/05/23 09:04:42, Ilya Sherman wrote: > nit: Can this be passed by const-reference? No, we can not, because buzz::XmlParseContext::ResolveQName() is not a const function. Which should be const I think. https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... File components/autofill/browser/autofill_xml_parser.h (right): https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autofill_xml_parser.h:103: const char** attrs, On 2013/05/23 09:04:42, Ilya Sherman wrote: > Please document that |attrs| will be modified by calling this method. Nope, the function only changes |attrs| itself (++attrs), and it will not be carried out of the function. However, the function could do this to change the the content it points to: *attrs = "foo"; To prevent that, we should define it as "const char* const* attrs" not just "const char** attrs". buzz::XmlParseHandler should do this as well. Const correctness is important but it is tricky to get it right, and easily get confused by these: char** a; const char** a; char* const* a; char** const a; const char* const* a; const chat* const* const a; ... ...
Hi Evan, Since Ilya is OOO, can you please take a look? On 2013/05/24 00:45:10, benquan wrote: > https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... > File components/autofill/browser/autocheckout_page_meta_data.h (right): > > https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... > components/autofill/browser/autocheckout_page_meta_data.h:42: // Elements have > to be clicked in order, and may wait between clicks. > On 2013/05/23 09:04:42, Ilya Sherman wrote: > > nit: No waiting. > > Done. > > https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... > components/autofill/browser/autocheckout_page_meta_data.h:43: > std::vector<WebElementDescriptor> click_elements_before_formfill; > On 2013/05/23 09:04:42, Ilya Sherman wrote: > > nit: "formfill" -> "form_fill" or something like that > > Done. > > https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... > components/autofill/browser/autocheckout_page_meta_data.h:46: // > page_advance_button. > On 2013/05/23 09:04:42, Ilya Sherman wrote: > > Why not just make the page_advance_button always be the last element that > needs > > to be clicked after filling the form? > > page_advance_button is already in the proto and our fieldmapping files. We could > deprecate it and use this instead later. > > https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... > components/autofill/browser/autocheckout_page_meta_data.h:47: // Elements have > to be clicked in order, and may wait between clicks. > On 2013/05/23 09:04:42, Ilya Sherman wrote: > > nit: No waiting. > > Done. > > https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... > components/autofill/browser/autocheckout_page_meta_data.h:48: > std::vector<WebElementDescriptor> click_elements_after_formfill; > On 2013/05/23 09:04:42, Ilya Sherman wrote: > > nit: "formfill" -> "form_fill" or something like that > > Done. > > https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... > File components/autofill/browser/autofill_xml_parser.cc (right): > > https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... > components/autofill/browser/autofill_xml_parser.cc:141: current_click_element_ = > &click_elements->back(); > On 2013/05/23 09:04:42, Ilya Sherman wrote: > > This doesn't look like it's actually parsed anything... > > renamed > > https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... > components/autofill/browser/autofill_xml_parser.cc:145: buzz::XmlParseContext* > context, > On 2013/05/23 09:04:42, Ilya Sherman wrote: > > nit: Can this be passed by const-reference? > > No, we can not, because buzz::XmlParseContext::ResolveQName() is not a const > function. Which should be const I think. > > https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... > File components/autofill/browser/autofill_xml_parser.h (right): > > https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... > components/autofill/browser/autofill_xml_parser.h:103: const char** attrs, > On 2013/05/23 09:04:42, Ilya Sherman wrote: > > Please document that |attrs| will be modified by calling this method. > > Nope, the function only changes |attrs| itself (++attrs), and it will not be > carried out of the function. > > However, the function could do this to change the the content it points to: > *attrs = "foo"; > > To prevent that, we should define it as "const char* const* attrs" not just > "const char** attrs". buzz::XmlParseHandler should do this as well. > > Const correctness is important but it is tricky to get it right, and easily get > confused by these: > > char** a; > const char** a; > char* const* a; > char** const a; > const char* const* a; > const chat* const* const a; > ... ...
Ping...
I think you should wait for Ilya to get back, because I don't have the slightest context for this block of code. https://codereview.chromium.org/15487004/diff/14001/components/autofill/brows... File components/autofill/browser/autofill_xml_parser.cc (right): https://codereview.chromium.org/15487004/diff/14001/components/autofill/brows... components/autofill/browser/autofill_xml_parser.cc:169: return element_descriptor; doesn't look like this return value is ever used?
Ilya, PTAL https://codereview.chromium.org/15487004/diff/14001/components/autofill/brows... File components/autofill/browser/autofill_xml_parser.cc (right): https://codereview.chromium.org/15487004/diff/14001/components/autofill/brows... components/autofill/browser/autofill_xml_parser.cc:169: return element_descriptor; On 2013/05/24 23:03:32, Evan Stade wrote: > doesn't look like this return value is ever used? Done.
On 2013/05/30 22:26:53, benquan wrote: > Ilya, PTAL > > https://codereview.chromium.org/15487004/diff/14001/components/autofill/brows... > File components/autofill/browser/autofill_xml_parser.cc (right): > > https://codereview.chromium.org/15487004/diff/14001/components/autofill/brows... > components/autofill/browser/autofill_xml_parser.cc:169: return > element_descriptor; > On 2013/05/24 23:03:32, Evan Stade wrote: > > doesn't look like this return value is ever used? > > Done. Ping...
https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... File components/autofill/browser/autocheckout_page_meta_data.h (right): https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autocheckout_page_meta_data.h:46: // page_advance_button. On 2013/05/24 00:45:11, benquan wrote: > On 2013/05/23 09:04:42, Ilya Sherman wrote: > > Why not just make the page_advance_button always be the last element that > needs > > to be clicked after filling the form? > > page_advance_button is already in the proto and our fieldmapping files. We could > deprecate it and use this instead later. That affects parsing the XML, but not the structure of the AutocheckoutPageMetaData class. https://codereview.chromium.org/15487004/diff/20001/components/autofill/brows... File components/autofill/browser/autofill_xml_parser.cc (right): https://codereview.chromium.org/15487004/diff/20001/components/autofill/brows... components/autofill/browser/autofill_xml_parser.cc:44: page_meta_data_(page_meta_data) { Make sure to initialize |current_click_element_| here. https://codereview.chromium.org/15487004/diff/20001/components/autofill/brows... components/autofill/browser/autofill_xml_parser.cc:124: // |attrs| is a NULL-terminated list of (attribute, value) pairs. nit: This comment should be moved into ParseElementDescriptor() as well. https://codereview.chromium.org/15487004/diff/20001/components/autofill/brows... components/autofill/browser/autofill_xml_parser.cc:141: current_click_element_ = &click_elements->back(); IMO it would be clearer to just inline this code in the two places where this method is currently called, as "HandleClickElement()" is a pretty vague name relative to what this method does, and the body is extremely short.
ahutter@, can you comment on make the page_advance_button always be the last element that needs to be clicked after filling the form? https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... File components/autofill/browser/autocheckout_page_meta_data.h (right): https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autocheckout_page_meta_data.h:46: // page_advance_button. On 2013/05/31 23:14:49, Ilya Sherman wrote: > On 2013/05/24 00:45:11, benquan wrote: > > On 2013/05/23 09:04:42, Ilya Sherman wrote: > > > Why not just make the page_advance_button always be the last element that > > needs > > > to be clicked after filling the form? > > > > page_advance_button is already in the proto and our fieldmapping files. We > could > > deprecate it and use this instead later. > > That affects parsing the XML, but not the structure of the > AutocheckoutPageMetaData class. We do expect page navigation when click on page_advance_button, if it doesn't we report an error. Oppositely, we do not expect page navigation when click elements in click_elements_after_formfill. If the merchant has no separated checkout confirm page, we may need to fill forms, and click some elements, but we do not proceed the page, if we add proceed_element_descriptor to be the last element in click_elements_after_formfill, we should have another flag to tell if the last element should proceed to next step. https://codereview.chromium.org/15487004/diff/20001/components/autofill/brows... File components/autofill/browser/autofill_xml_parser.cc (right): https://codereview.chromium.org/15487004/diff/20001/components/autofill/brows... components/autofill/browser/autofill_xml_parser.cc:44: page_meta_data_(page_meta_data) { On 2013/05/31 23:14:49, Ilya Sherman wrote: > Make sure to initialize |current_click_element_| here. Done. https://codereview.chromium.org/15487004/diff/20001/components/autofill/brows... components/autofill/browser/autofill_xml_parser.cc:124: // |attrs| is a NULL-terminated list of (attribute, value) pairs. On 2013/05/31 23:14:49, Ilya Sherman wrote: > nit: This comment should be moved into ParseElementDescriptor() as well. Done. https://codereview.chromium.org/15487004/diff/20001/components/autofill/brows... components/autofill/browser/autofill_xml_parser.cc:141: current_click_element_ = &click_elements->back(); On 2013/05/31 23:14:49, Ilya Sherman wrote: > IMO it would be clearer to just inline this code in the two places where this > method is currently called, as "HandleClickElement()" is a pretty vague name > relative to what this method does, and the body is extremely short. Done.
LGTM with a last few comments. Thanks. https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... File components/autofill/browser/autocheckout_page_meta_data.h (right): https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autocheckout_page_meta_data.h:46: // page_advance_button. On 2013/06/01 01:08:39, benquan wrote: > On 2013/05/31 23:14:49, Ilya Sherman wrote: > > On 2013/05/24 00:45:11, benquan wrote: > > > On 2013/05/23 09:04:42, Ilya Sherman wrote: > > > > Why not just make the page_advance_button always be the last element that > > > needs > > > > to be clicked after filling the form? > > > > > > page_advance_button is already in the proto and our fieldmapping files. We > > could > > > deprecate it and use this instead later. > > > > That affects parsing the XML, but not the structure of the > > AutocheckoutPageMetaData class. > > We do expect page navigation when click on page_advance_button, if it doesn't we > report an error. Oppositely, we do not expect page navigation when click > elements in click_elements_after_formfill. > > If the merchant has no separated checkout confirm page, we may need to fill > forms, and click some elements, but we do not proceed the page, if we add > proceed_element_descriptor to be the last element in > click_elements_after_formfill, we should have another flag to tell if the last > element should proceed to next step. Hmm, fair enough. Please add this explanation to the documentation, so that somebody like me doesn't come along later and try to refactor the |page_advance_button_| away. https://codereview.chromium.org/15487004/diff/29001/components/autofill/brows... File components/autofill/browser/autofill_xml_parser.h (right): https://codereview.chromium.org/15487004/diff/29001/components/autofill/brows... components/autofill/browser/autofill_xml_parser.h:95: void HandleClickElement(std::vector<WebElementDescriptor>* click_elements); This method declaration should be removed now.
https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... File components/autofill/browser/autocheckout_page_meta_data.h (right): https://codereview.chromium.org/15487004/diff/8001/components/autofill/browse... components/autofill/browser/autocheckout_page_meta_data.h:46: // page_advance_button. On 2013/06/01 01:22:59, Ilya Sherman wrote: > On 2013/06/01 01:08:39, benquan wrote: > > On 2013/05/31 23:14:49, Ilya Sherman wrote: > > > On 2013/05/24 00:45:11, benquan wrote: > > > > On 2013/05/23 09:04:42, Ilya Sherman wrote: > > > > > Why not just make the page_advance_button always be the last element > that > > > > needs > > > > > to be clicked after filling the form? > > > > > > > > page_advance_button is already in the proto and our fieldmapping files. We > > > could > > > > deprecate it and use this instead later. > > > > > > That affects parsing the XML, but not the structure of the > > > AutocheckoutPageMetaData class. > > > > We do expect page navigation when click on page_advance_button, if it doesn't > we > > report an error. Oppositely, we do not expect page navigation when click > > elements in click_elements_after_formfill. > > > > If the merchant has no separated checkout confirm page, we may need to fill > > forms, and click some elements, but we do not proceed the page, if we add > > proceed_element_descriptor to be the last element in > > click_elements_after_formfill, we should have another flag to tell if the last > > element should proceed to next step. > > Hmm, fair enough. Please add this explanation to the documentation, so that > somebody like me doesn't come along later and try to refactor the > |page_advance_button_| away. Done. https://codereview.chromium.org/15487004/diff/29001/components/autofill/brows... File components/autofill/browser/autofill_xml_parser.h (right): https://codereview.chromium.org/15487004/diff/29001/components/autofill/brows... components/autofill/browser/autofill_xml_parser.h:95: void HandleClickElement(std::vector<WebElementDescriptor>* click_elements); On 2013/06/01 01:22:59, Ilya Sherman wrote: > This method declaration should be removed now. Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benquan@chromium.org/15487004/35001
Message was sent while issue was closed.
Change committed as 203854 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
