Umbrella bug for FindinPage cross-domain iframes |
||||||||
Issue descriptionUse this to track all CLs that relate to work with FindinPage for cross-domain iframe support. Could be refactor, metrics setup, and actual code changes.
,
Oct 26
kariahda@ Can i get approval to cherry-pick the above CL into M-71? It is a UKM for the FindInPage cross-domain iframes that I will be working on and would like informative metrics established for M71. It has gotten approval from the Metrics and privacy team.
,
Oct 26
This bug requires manual review: DEPS changes referenced in bugdroid comments. Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 30
Is there a way to verify this is working as intended before merge?
,
Oct 30
Unittests were written that verify the UKM recording performs as intentioned.
,
Oct 30
To clarify, those tests are in the CL.
,
Oct 30
Great, thanks. Approved.
,
Oct 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0503734e70114e36c98a37f546b13a5b17280cd0 commit 0503734e70114e36c98a37f546b13a5b17280cd0 Author: Chris Lu <thegreenfrog@chromium.org> Date: Tue Oct 30 22:29:45 2018 [ios] Add UKM metric for Find in Page actions This metric will track which URLs users are making Find in Page search requests in addition to whether or not matches were found. Having this metric will help with testing and measuring effects of changes to Find in Page logic. UKM review doc: https://docs.google.com/document/d/1qb4aZo1CAgSqo6OyBNAc1krAOQgNqYnrm6lDlwfbcZk/edit?usp=sharing Bug: 896867 Change-Id: Ie0f78da7b586ec2292405fa75eca1f4cf0efe3b9 Reviewed-on: https://chromium-review.googlesource.com/c/1289696 Commit-Queue: Chris Lu <thegreenfrog@chromium.org> Reviewed-by: Eugene But <eugenebut@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Reviewed-by: Mike Dougherty <michaeldo@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#602812}(cherry picked from commit e6e2df6817fe5982c79d732332e578e1817a1511) Reviewed-on: https://chromium-review.googlesource.com/c/1309139 Reviewed-by: Chris Lu <thegreenfrog@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#423} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/0503734e70114e36c98a37f546b13a5b17280cd0/ios/chrome/browser/DEPS [modify] https://crrev.com/0503734e70114e36c98a37f546b13a5b17280cd0/ios/chrome/browser/find_in_page/BUILD.gn [modify] https://crrev.com/0503734e70114e36c98a37f546b13a5b17280cd0/ios/chrome/browser/find_in_page/find_in_page_controller.mm [add] https://crrev.com/0503734e70114e36c98a37f546b13a5b17280cd0/ios/chrome/browser/find_in_page/find_in_page_controller_unittest.mm [modify] https://crrev.com/0503734e70114e36c98a37f546b13a5b17280cd0/tools/metrics/ukm/ukm.xml
,
Oct 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0503734e70114e36c98a37f546b13a5b17280cd0 Commit: 0503734e70114e36c98a37f546b13a5b17280cd0 Author: thegreenfrog@chromium.org Commiter: thegreenfrog@chromium.org Date: 2018-10-30 22:29:45 +0000 UTC [ios] Add UKM metric for Find in Page actions This metric will track which URLs users are making Find in Page search requests in addition to whether or not matches were found. Having this metric will help with testing and measuring effects of changes to Find in Page logic. UKM review doc: https://docs.google.com/document/d/1qb4aZo1CAgSqo6OyBNAc1krAOQgNqYnrm6lDlwfbcZk/edit?usp=sharing Bug: 896867 Change-Id: Ie0f78da7b586ec2292405fa75eca1f4cf0efe3b9 Reviewed-on: https://chromium-review.googlesource.com/c/1289696 Commit-Queue: Chris Lu <thegreenfrog@chromium.org> Reviewed-by: Eugene But <eugenebut@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Reviewed-by: Mike Dougherty <michaeldo@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#602812}(cherry picked from commit e6e2df6817fe5982c79d732332e578e1817a1511) Reviewed-on: https://chromium-review.googlesource.com/c/1309139 Reviewed-by: Chris Lu <thegreenfrog@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#423} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/77905ac0652a3187769ee51328a1308d23510f6a commit 77905ac0652a3187769ee51328a1308d23510f6a Author: Chris Lu <thegreenfrog@chromium.org> Date: Thu Dec 06 20:46:51 2018 [ios] feature flag for find in page iframe Bug: 896867 Change-Id: Ia83521278052848fe57f3a99f91e1ad9dd51deac Reviewed-on: https://chromium-review.googlesource.com/c/1364334 Reviewed-by: Rohit Rao <rohitrao@chromium.org> Commit-Queue: Chris Lu <thegreenfrog@chromium.org> Cr-Commit-Position: refs/heads/master@{#614482} [modify] https://crrev.com/77905ac0652a3187769ee51328a1308d23510f6a/ios/chrome/browser/BUILD.gn [modify] https://crrev.com/77905ac0652a3187769ee51328a1308d23510f6a/ios/chrome/browser/about_flags.mm [modify] https://crrev.com/77905ac0652a3187769ee51328a1308d23510f6a/ios/chrome/browser/find_in_page/BUILD.gn [add] https://crrev.com/77905ac0652a3187769ee51328a1308d23510f6a/ios/chrome/browser/find_in_page/features.h [add] https://crrev.com/77905ac0652a3187769ee51328a1308d23510f6a/ios/chrome/browser/find_in_page/features.mm [modify] https://crrev.com/77905ac0652a3187769ee51328a1308d23510f6a/ios/chrome/browser/ios_chrome_flag_descriptions.cc [modify] https://crrev.com/77905ac0652a3187769ee51328a1308d23510f6a/ios/chrome/browser/ios_chrome_flag_descriptions.h
,
Dec 14
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1a37e436b476fcd5def0056f4774d2528ec2a8dc commit 1a37e436b476fcd5def0056f4774d2528ec2a8dc Author: Chris Lu <thegreenfrog@chromium.org> Date: Wed Dec 19 19:23:37 2018 [ios] Copy Find in Page JS from //ios/browser to //ios/web Copy entire JS logic so that diffing changes to code that already existed is easier for reviewers. Add bug numbers back to old todos. Bug: 896867 Change-Id: I016cfbb2c7a921f33440be2f83fdb6147f69ad72 Reviewed-on: https://chromium-review.googlesource.com/c/1383578 Commit-Queue: Chris Lu <thegreenfrog@chromium.org> Reviewed-by: Eugene But <eugenebut@chromium.org> Cr-Commit-Position: refs/heads/master@{#617904} [modify] https://crrev.com/1a37e436b476fcd5def0056f4774d2528ec2a8dc/ios/chrome/browser/find_in_page/resources/find_in_page.js [modify] https://crrev.com/1a37e436b476fcd5def0056f4774d2528ec2a8dc/ios/web/web_state/js/resources/find_in_page.js
,
Jan 7
,
Jan 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a79e6dd15ef4f40a89be2f7a31a1467f474a94ff commit a79e6dd15ef4f40a89be2f7a31a1467f474a94ff Author: Chris Lu <thegreenfrog@chromium.org> Date: Wed Jan 09 00:06:15 2019 [ios] Allow WebFrame messaging to return results of value 0. Currently, message.js only sets the response 'result' value if the return value of a JavaScript function call isn't false, 0, null or undefined. Set the result only if it is not undefined. Bug: 896867 Change-Id: I9f2211f71ffe9c70b9b4ce85eb949667baa350a6 Reviewed-on: https://chromium-review.googlesource.com/c/1399228 Commit-Queue: Chris Lu <thegreenfrog@chromium.org> Reviewed-by: Eugene But <eugenebut@chromium.org> Cr-Commit-Position: refs/heads/master@{#620954} [modify] https://crrev.com/a79e6dd15ef4f40a89be2f7a31a1467f474a94ff/ios/web/web_state/js/resources/message.js
,
Jan 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5968f8fc2c5c9e3d830fe48d5a9bbd1b3ac5489f commit 5968f8fc2c5c9e3d830fe48d5a9bbd1b3ac5489f Author: Chris Lu <thegreenfrog@chromium.org> Date: Tue Jan 15 00:47:59 2019 [ios] Refactor //ios/web find_in_page.js search Changes highlightWord nomenclature to findString, and removes logic that searches through all child frames. Refactor pumpSearch to not highlight first result. findString and pumpSearch also only returns numerical value of matches since decision which frame to highligh/scroll to is done by manager. Bug: 896867, 895529 Change-Id: Ib71ffaffda96e20a0e0721df0085476e998ae017 Reviewed-on: https://chromium-review.googlesource.com/c/1387045 Reviewed-by: Eugene But <eugenebut@chromium.org> Reviewed-by: Mike Dougherty <michaeldo@chromium.org> Commit-Queue: Chris Lu <thegreenfrog@chromium.org> Cr-Commit-Position: refs/heads/master@{#622662} [modify] https://crrev.com/5968f8fc2c5c9e3d830fe48d5a9bbd1b3ac5489f/ios/web/BUILD.gn [add] https://crrev.com/5968f8fc2c5c9e3d830fe48d5a9bbd1b3ac5489f/ios/web/web_state/js/find_in_page_unittest.mm [modify] https://crrev.com/5968f8fc2c5c9e3d830fe48d5a9bbd1b3ac5489f/ios/web/web_state/js/resources/find_in_page.js
,
Jan 15
The last CL broke the IOS builders: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8924306034326802912/+/steps/ios_web_unittests__iPhone_5s_iOS_10.3__on_Mac-10.13.6/0/logs/FindInPageWebJsTest.FindIFrameText/0 FindInPageWebJsTest.FindIFrameText: ../../ios/web/web_state/js/find_in_page_unittest.mm:133: Failure Value of: WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^{ return message_received; }) Actual: false Expected: true Stack trace: 0 ios_web_unittests 0x000000010f6c00e5 base::debug::StackTrace::StackTrace() + 37 1 ios_web_unittests 0x000000010e5ad2a0 StackTraceGetter::CurrentStackTrace(int, int) + 64 2 ios_web_unittests 0x000000010e5b5187 testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop(int) + 71 3 ios_web_unittests 0x000000010e5b4d13 testing::internal::AssertHelper::operator=(testing::Message const&) const + 131 4 ios_web_unittests 0x000000010e3d6dbf web::FindInPageWebJsTest_FindIFrameText_Test::TestBody() + 2 Reverting...
,
Jan 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7c893b0aafa24f476849fc0d4d61cd450f5255c4 commit 7c893b0aafa24f476849fc0d4d61cd450f5255c4 Author: Dominic Battré <battre@chromium.org> Date: Tue Jan 15 09:12:39 2019 Revert "[ios] Refactor //ios/web find_in_page.js search" This reverts commit 5968f8fc2c5c9e3d830fe48d5a9bbd1b3ac5489f. Reason for revert: Broken test, see crbug.com/896867 Original change's description: > [ios] Refactor //ios/web find_in_page.js search > > Changes highlightWord nomenclature to findString, and removes logic that > searches through all child frames. Refactor pumpSearch to not highlight > first result. findString and pumpSearch also only returns numerical value > of matches since decision which frame to highligh/scroll to is done by > manager. > > Bug: 896867, 895529 > Change-Id: Ib71ffaffda96e20a0e0721df0085476e998ae017 > Reviewed-on: https://chromium-review.googlesource.com/c/1387045 > Reviewed-by: Eugene But <eugenebut@chromium.org> > Reviewed-by: Mike Dougherty <michaeldo@chromium.org> > Commit-Queue: Chris Lu <thegreenfrog@chromium.org> > Cr-Commit-Position: refs/heads/master@{#622662} TBR=eugenebut@chromium.org,michaeldo@chromium.org,thegreenfrog@chromium.org Change-Id: I9c5339b5561c152c3c90d68114bfb24813053bf0 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 896867, 895529 Reviewed-on: https://chromium-review.googlesource.com/c/1411336 Reviewed-by: Dominic Battré <battre@chromium.org> Commit-Queue: Dominic Battré <battre@chromium.org> Cr-Commit-Position: refs/heads/master@{#622791} [modify] https://crrev.com/7c893b0aafa24f476849fc0d4d61cd450f5255c4/ios/web/BUILD.gn [delete] https://crrev.com/630b98c03608a241cfca63c1c47576c8c178b5f7/ios/web/web_state/js/find_in_page_unittest.mm [modify] https://crrev.com/7c893b0aafa24f476849fc0d4d61cd450f5255c4/ios/web/web_state/js/resources/find_in_page.js
,
Jan 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2162251259f92f0b95f0bc25a570357e4e5fb509 commit 2162251259f92f0b95f0bc25a570357e4e5fb509 Author: Chris Lu <thegreenfrog@chromium.org> Date: Wed Jan 16 01:59:14 2019 [ios] Refactor //ios/web find_in_page.js search Changes highlightWord nomenclature to findString, and removes logic that searches through all child frames. Refactor pumpSearch to not highlight first result. findString and pumpSearch also only returns numerical value of matches since decision which frame to highligh/scroll to is done by manager. Bug: 896867, 895529 Change-Id: I7693a073181b5f9c6ec8b9a0ed21b9f05012793f Reviewed-on: https://chromium-review.googlesource.com/c/1412593 Reviewed-by: Chris Lu <thegreenfrog@chromium.org> Reviewed-by: Eugene But <eugenebut@chromium.org> Commit-Queue: Chris Lu <thegreenfrog@chromium.org> Cr-Commit-Position: refs/heads/master@{#623017} [modify] https://crrev.com/2162251259f92f0b95f0bc25a570357e4e5fb509/ios/web/BUILD.gn [add] https://crrev.com/2162251259f92f0b95f0bc25a570357e4e5fb509/ios/web/web_state/js/find_in_page_unittest.mm [modify] https://crrev.com/2162251259f92f0b95f0bc25a570357e4e5fb509/ios/web/web_state/js/resources/find_in_page.js
,
Jan 18
(4 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c114ad077c6534861b807ce6c2f7bbab4fb7a41e commit c114ad077c6534861b807ce6c2f7bbab4fb7a41e Author: Chris Lu <thegreenfrog@chromium.org> Date: Fri Jan 18 22:53:38 2019 [ios] Add FindInPageManagerImpl class Create new FindInPageManagerImpl class. Implement constructor/destructor and WebStateUserData and WebStateObserver methods. Bug: 896867 Change-Id: I016e4fe3e1f43d5163a28eb70ff5ecffc91df354 Reviewed-on: https://chromium-review.googlesource.com/c/1399544 Commit-Queue: Chris Lu <thegreenfrog@chromium.org> Reviewed-by: Eugene But <eugenebut@chromium.org> Reviewed-by: Mike Dougherty <michaeldo@chromium.org> Cr-Commit-Position: refs/heads/master@{#624331} [modify] https://crrev.com/c114ad077c6534861b807ce6c2f7bbab4fb7a41e/ios/web/web_state/BUILD.gn [add] https://crrev.com/c114ad077c6534861b807ce6c2f7bbab4fb7a41e/ios/web/web_state/find_in_page/find_in_page_manager_impl.h [add] https://crrev.com/c114ad077c6534861b807ce6c2f7bbab4fb7a41e/ios/web/web_state/find_in_page/find_in_page_manager_impl.mm |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by bugdroid1@chromium.org
, Oct 25