[CherryPick][iOS] Handle JavaScript Responses in CRWTextFragmentsHandler
Set-up response creation in JavaScript and handling in
CRWTextFragmentsHandler, allowing the collection of success/failure
metrics and measurement of the JavaScript library's rate of success.
(cherry picked from commit ff013a43d963dcc357eb8b67c1a448fc689629d8)
Bug: 1131107
Change-Id: I01eea5d85e2827a496b011d116592bebb74bd92e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2448683
Commit-Queue: Sebastien Lalancette <[email protected]>
Reviewed-by: Eugene But <[email protected]>
Reviewed-by: Tommy Martino <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#814791}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2475573
Reviewed-by: Sebastien Lalancette <[email protected]>
Cr-Commit-Position: refs/branch-heads/4280@{#411}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/ios/web/navigation/crw_text_fragments_handler.h b/ios/web/navigation/crw_text_fragments_handler.h
index bd509e2..2977137 100644
--- a/ios/web/navigation/crw_text_fragments_handler.h
+++ b/ios/web/navigation/crw_text_fragments_handler.h
@@ -20,6 +20,7 @@
// WebStates' loaded URLs.
@interface CRWTextFragmentsHandler : CRWWebViewHandler
+// Initializes a handler with a |delegate| to retrieve the current WebState.
- (instancetype)initWithDelegate:(id<CRWWebViewHandlerDelegate>)delegate;
// Checks the WebState's destination URL for Text Fragments. If found, searches
diff --git a/ios/web/navigation/crw_text_fragments_handler.mm b/ios/web/navigation/crw_text_fragments_handler.mm
index 83ca273..f127c57 100644
--- a/ios/web/navigation/crw_text_fragments_handler.mm
+++ b/ios/web/navigation/crw_text_fragments_handler.mm
@@ -9,17 +9,26 @@
#import "base/strings/utf_string_conversions.h"
#import "ios/web/common/features.h"
#import "ios/web/navigation/text_fragments_utils.h"
+#import "ios/web/public/js_messaging/web_frame.h"
#import "ios/web/public/navigation/navigation_context.h"
#import "ios/web/public/navigation/referrer.h"
-#import "ios/web/web_state/web_state_impl.h"
-
#import "ios/web/web_state/ui/crw_web_view_handler_delegate.h"
+#import "ios/web/web_state/web_state_impl.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
-@interface CRWTextFragmentsHandler ()
+namespace {
+
+const char kScriptCommandPrefix[] = "textFragments";
+const char kScriptResponseCommand[] = "textFragments.response";
+
+} // namespace
+
+@interface CRWTextFragmentsHandler () {
+ std::unique_ptr<web::WebState::ScriptCommandSubscription> _subscription;
+}
@property(nonatomic, weak) id<CRWWebViewHandlerDelegate> delegate;
@@ -31,8 +40,24 @@
@implementation CRWTextFragmentsHandler
- (instancetype)initWithDelegate:(id<CRWWebViewHandlerDelegate>)delegate {
+ DCHECK(delegate);
if (self = [super init]) {
_delegate = delegate;
+
+ if (base::FeatureList::IsEnabled(web::features::kScrollToTextIOS) &&
+ self.webStateImpl) {
+ __weak __typeof(self) weakSelf = self;
+ const web::WebState::ScriptCommandCallback callback = base::BindRepeating(
+ ^(const base::DictionaryValue& message, const GURL& page_url,
+ bool interacted, web::WebFrame* sender_frame) {
+ if (sender_frame && sender_frame->IsMainFrame()) {
+ [weakSelf didReceiveJavaScriptResponse:message];
+ }
+ });
+
+ _subscription = self.webStateImpl->AddScriptCommandCallback(
+ callback, kScriptCommandPrefix);
+ }
}
return self;
@@ -50,6 +75,7 @@
if (parsedFragments.type() == base::Value::Type::NONE)
return;
+ // TODO (crbug.com/1099268): Log the origin and number of fragments metrics.
std::string fragmentParam;
base::JSONWriter::Write(parsedFragments, &fragmentParam);
@@ -89,4 +115,13 @@
return [self.delegate webStateImplForWebViewHandler:self];
}
+- (void)didReceiveJavaScriptResponse:(const base::DictionaryValue&)response {
+ const std::string* command = response.FindStringKey("command");
+ if (!command || *command != kScriptResponseCommand) {
+ return;
+ }
+
+ // TODO (crbug.com/1099268): Log the success/failure metric.
+}
+
@end
diff --git a/ios/web/navigation/crw_text_fragments_handler_unittest.mm b/ios/web/navigation/crw_text_fragments_handler_unittest.mm
index 9b130c0..463440c 100644
--- a/ios/web/navigation/crw_text_fragments_handler_unittest.mm
+++ b/ios/web/navigation/crw_text_fragments_handler_unittest.mm
@@ -43,6 +43,23 @@
MOCK_METHOD1(ExecuteJavaScript, void(const base::string16&));
MOCK_CONST_METHOD0(GetLastCommittedURL, const GURL&());
+
+ std::unique_ptr<web::WebState::ScriptCommandSubscription>
+ AddScriptCommandCallback(const web::WebState::ScriptCommandCallback& callback,
+ const std::string& command_prefix) override {
+ last_callback_ = callback;
+ last_command_prefix_ = command_prefix;
+ return nil;
+ }
+
+ web::WebState::ScriptCommandCallback last_callback() {
+ return last_callback_;
+ }
+ const std::string last_command_prefix() { return last_command_prefix_; }
+
+ private:
+ web::WebState::ScriptCommandCallback last_callback_;
+ std::string last_command_prefix_;
};
class CRWTextFragmentsHandlerTest : public web::WebTest {
@@ -109,6 +126,11 @@
EXPECT_CALL(*web_state_, ExecuteJavaScript(expected_javascript)).Times(1);
[handler processTextFragmentsWithContext:&context_ referrer:Referrer()];
+
+ // Verify that a command callback was added with the right prefix.
+ EXPECT_NE(web::WebState::ScriptCommandCallback(),
+ web_state_->last_callback());
+ EXPECT_EQ("textFragments", web_state_->last_command_prefix());
}
// Tests that the handler will not execute JavaScript if the scroll to text
@@ -123,6 +145,10 @@
EXPECT_CALL(*web_state_, GetLastCommittedURL()).Times(0);
[handler processTextFragmentsWithContext:&context_ referrer:Referrer()];
+
+ // Verify that no callback was set when the flag is disabled.
+ EXPECT_EQ(web::WebState::ScriptCommandCallback(),
+ web_state_->last_callback());
}
// Tests that the handler will not execute JavaScript if the WebState has an
diff --git a/ios/web/web_state/js/resources/text_fragments.js b/ios/web/web_state/js/resources/text_fragments.js
index 2c775d5..d5bd4405 100644
--- a/ios/web/web_state/js/resources/text_fragments.js
+++ b/ios/web/web_state/js/resources/text_fragments.js
@@ -37,27 +37,38 @@
* Does the actual work for handleTextFragments.
*/
const doHandleTextFragments = function(fragments, scroll) {
- let marks = [];
+ const marks = [];
+ let successCount = 0;
for (const fragment of fragments) {
// Process the fragments, then filter out any that evaluate to false.
- let newMarks = utils.processTextFragmentDirective(fragment)
+ const newMarks = utils.processTextFragmentDirective(fragment)
.filter((mark) => { return !!mark });
- if (Array.isArray(newMarks))
+ if (Array.isArray(newMarks)) {
+ if (newMarks.length > 0) {
+ ++successCount;
+ }
+
marks.push(...newMarks);
+ }
}
if (scroll && marks.length > 0)
utils.scrollElementIntoView(marks[0]);
- // TODO(crbug.com/1099268): Count successes/failures above and log metrics
-
for (const mark of marks) {
mark.addEventListener("click", () => {
utils.removeMarks(marks);
});
}
- }
+ __gCrWeb.message.invokeOnHost({
+ command: 'textFragments.response',
+ result: {
+ successCount: successCount,
+ fragmentsCount: fragments.length
+ }
+ });
+ }
})();
