Fix for problem with activating find-in-page match via find tickbar.

Fix for problem with activating find-in-page match via find tickbar.

The problem was that the TabFindHelper was throwing out certain valid find replies because the helper was only comparing them against the latest find request ID it knows about (which is usually the ID that valid replies will have).

This patch makes it so that this and another android-specific find helper now properly track the current find session ID (in addition to the latest request ID) so that they can accept all valid find replies relevant to the current find session.

BUG=605297
Committed: https://crrev.com/1cfca29de5756e4f3e5b5210d21cf6f4461d0aa8
Cr-Commit-Position: refs/heads/master@{#389603}

[paulmeyer, 2016-04-22 20:10:46]

Patchset #1 (id:1) has been deleted

[paulmeyer, 2016-04-22 20:12:24]

The CQ bit was checked by paulmeyer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-22 20:13:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1913543002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1913543002/20001

[paulmeyer, 2016-04-22 21:09:56]

paulmeyer@chromium.org changed reviewers:
+ nick@chromium.org

[paulmeyer, 2016-04-22 21:09:57]

+nick@ for review of changes to find_request_manager.cc

[paulmeyer, 2016-04-22 21:10:51]

paulmeyer@chromium.org changed reviewers:
+ sky@chromium.org

[paulmeyer, 2016-04-22 21:10:52]

+sky@ for OWNER review of chrome/

[paulmeyer, 2016-04-22 21:11:27]

paulmeyer@chromium.org changed reviewers:
+ sgurun@chromium.org

[paulmeyer, 2016-04-22 21:11:27]

+sgurun@ for OWNER review of android_webview/

[commit-bot: I haz the power, 2016-04-22 21:27:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-22 21:27:55]

Dry run: This issue passed the CQ dry run.

[sky, 2016-04-22 21:38:34]

I have to admit this doesn't make sense to me. Where are the ids documented?

[paulmeyer, 2016-04-25 15:49:39]

On 2016/04/22 21:38:34, sky wrote:
> I have to admit this doesn't make sense to me. Where are the ids documented?

I've added a section to my design doc for this. Take a look at the "Find IDs"
section:
https://docs.google.com/document/d/12S_6X2MWWLoyJslajcajL2ELU7zhodPxMOxa8Bg4W...

Hopefully that clarifies things. Please let me know if there is still anything
unclear in the documentation. :)

[sky, 2016-04-25 16:10:26]

Thanks for the update. The ids should be documented in code some
where. Perhaps in WebContents::Fint.

On Mon, Apr 25, 2016 at 8:49 AM,  <paulmeyer@chromium.org> wrote:
> On 2016/04/22 21:38:34, sky wrote:
>> I have to admit this doesn't make sense to me. Where are the ids
>> documented?
>
> I've added a section to my design doc for this. Take a look at the "Find
> IDs"
> section:
>
https://docs.google.com/document/d/12S_6X2MWWLoyJslajcajL2ELU7zhodPxMOxa8Bg4W...
>
> Hopefully that clarifies things. Please let me know if there is still
> anything
> unclear in the documentation. :)
>
> https://codereview.chromium.org/1913543002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[paulmeyer, 2016-04-25 17:52:15]

On 2016/04/25 16:10:26, sky wrote:
> Thanks for the update. The ids should be documented in code some
> where. Perhaps in WebContents::Fint.
> 
> On Mon, Apr 25, 2016 at 8:49 AM,  <mailto:paulmeyer@chromium.org> wrote:
> > On 2016/04/22 21:38:34, sky wrote:
> >> I have to admit this doesn't make sense to me. Where are the ids
> >> documented?
> >
> > I've added a section to my design doc for this. Take a look at the "Find
> > IDs"
> > section:
> >
>
https://docs.google.com/document/d/12S_6X2MWWLoyJslajcajL2ELU7zhodPxMOxa8Bg4W...
> >
> > Hopefully that clarifies things. Please let me know if there is still
> > anything
> > unclear in the documentation. :)
> >
> > https://codereview.chromium.org/1913543002/
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.
> 

It is documented in the code in find_request_manager.cc: 
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fi...

[ncarter (slow), 2016-04-25 18:02:38]

lgtm

https://codereview.chromium.org/1913543002/diff/20001/chrome/browser/ui/find_...
File chrome/browser/ui/find_bar/find_tab_helper.cc (right):

https://codereview.chromium.org/1913543002/diff/20001/chrome/browser/ui/find_...
chrome/browser/ui/find_bar/find_tab_helper.cc:163: // requests from previous
sessiona. That way we won't act on stale results
"sessiona"

[sky, 2016-04-25 18:25:54]

Why wouldn't it be closer to the place where the api is, like in
WebContents as I asked for?

On Mon, Apr 25, 2016 at 10:52 AM,  <paulmeyer@chromium.org> wrote:
> On 2016/04/25 16:10:26, sky wrote:
>> Thanks for the update. The ids should be documented in code some
>> where. Perhaps in WebContents::Fint.
>>
>> On Mon, Apr 25, 2016 at 8:49 AM, <mailto:paulmeyer@chromium.org> wrote:
>> > On 2016/04/22 21:38:34, sky wrote:
>> >> I have to admit this doesn't make sense to me. Where are the ids
>> >> documented?
>> >
>> > I've added a section to my design doc for this. Take a look at the "Find
>> > IDs"
>> > section:
>> >
>>
>
https://docs.google.com/document/d/12S_6X2MWWLoyJslajcajL2ELU7zhodPxMOxa8Bg4W...
>> >
>> > Hopefully that clarifies things. Please let me know if there is still
>> > anything
>> > unclear in the documentation. :)
>> >
>> > https://codereview.chromium.org/1913543002/
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Chromium-reviews" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
> email
>> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>
> It is documented in the code in find_request_manager.cc:
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fi...
>
> https://codereview.chromium.org/1913543002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sky, 2016-04-25 18:26:40]

LGTM - the documentation can be straightened out separately.

[sgurun-gerrit only, 2016-04-25 21:25:08]

On 2016/04/25 18:26:40, sky wrote:
> LGTM - the documentation can be straightened out separately.

lgtm, it seems that we need a new webview webview test to make sure we catch the
problem introduced in the previous CL.

[paulmeyer, 2016-04-25 21:41:20]

The CQ bit was checked by paulmeyer@chromium.org

[paulmeyer, 2016-04-25 21:41:21]

The patchset sent to the CQ was uploaded after l-g-t-m from nick@chromium.org,
sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1913543002/#ps40001
(title: "Rebased and fixed typo in comment.")

[commit-bot: I haz the power, 2016-04-25 21:41:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1913543002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1913543002/40001

[commit-bot: I haz the power, 2016-04-25 23:26:09]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-04-25 23:28:07]

Description was changed from

==========
Fix for problem with activating find-in-page match via find tickbar.

The problem was that the TabFindHelper was throwing out certain valid find
replies because the helper was only comparing them against the latest find
request ID it knows about (which is usually the ID that valid replies will
have).

This patch makes it so that this and another android-specific find helper now
properly track the current find session ID (in addition to the latest request
ID) so that they can accept all valid find replies relevant to the current find
session.

BUG=605297
==========

to

==========
Fix for problem with activating find-in-page match via find tickbar.

The problem was that the TabFindHelper was throwing out certain valid find
replies because the helper was only comparing them against the latest find
request ID it knows about (which is usually the ID that valid replies will
have).

This patch makes it so that this and another android-specific find helper now
properly track the current find session ID (in addition to the latest request
ID) so that they can accept all valid find replies relevant to the current find
session.

BUG=605297

Committed: https://crrev.com/1cfca29de5756e4f3e5b5210d21cf6f4461d0aa8
Cr-Commit-Position: refs/heads/master@{#389603}
==========

[commit-bot: I haz the power, 2016-04-25 23:28:08]

Patchset 2 (id:??) landed as
https://crrev.com/1cfca29de5756e4f3e5b5210d21cf6f4461d0aa8
Cr-Commit-Position: refs/heads/master@{#389603}