Commit ff013a43 authored by Sebastien Lalancette's avatar Sebastien Lalancette Committed by Commit Bot

[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.

Bug: 1136043
Change-Id: I01eea5d85e2827a496b011d116592bebb74bd92e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2448683
Commit-Queue: Sebastien Lalancette <seblalancette@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarTommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814791}
parent 804c9bd5
......@@ -20,6 +20,7 @@ struct Referrer;
// 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
......
......@@ -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
......@@ -43,6 +43,23 @@ class MockWebStateImpl : public web::WebStateImpl {
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 @@ TEST_F(CRWTextFragmentsHandlerTest, ExecuteJavaScriptSuccess) {
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 @@ TEST_F(CRWTextFragmentsHandlerTest, FeatureDisabledFragmentsDisallowed) {
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
......
......@@ -37,27 +37,38 @@ const utils = goog.require(
* 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
}
});
}
})();
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment