Skip to content

Commit

Permalink
Revert "Remove __gCrWeb.windowId"
Browse files Browse the repository at this point in the history
This reverts commit 0660111.

Reason for revert: https://ci.chromium.org/p/chrome/builders/ci/device-builder/148009

Original change's description:
> Remove __gCrWeb.windowId
>
> __gCrWeb.windowId was used to ensure that JavaScript was executed on
> the intended webpage. This is important because some scripts may deal
> with user information. Such information is now all passed through
> APIs using WebKit's FrameInfo instance which also has the same
> guarantee because FrameInfo instances are tied to a particular
> navigation.
>
> Fixed: 905939
>
> Change-Id: I19a2e0a7a7ef0f737af99962ec41294c5cbbbd04
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4189036
> Reviewed-by: Ali Juma <ajuma@chromium.org>
> Reviewed-by: Sergio Collazos <sczs@chromium.org>
> Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1096265}

Change-Id: I474a5740180741ad9562085b5ab299f70a4cb95e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4188583
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Auto-Submit: Ginny Huang <ginnyhuang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1096288}
  • Loading branch information
Ginny Huang authored and Chromium LUCI CQ committed Jan 24, 2023
1 parent 341fedf commit f860c14
Show file tree
Hide file tree
Showing 24 changed files with 466 additions and 48 deletions.
14 changes: 7 additions & 7 deletions ios/chrome/browser/passwords/password_controller_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1105,23 +1105,23 @@ PasswordForm MakeSimpleForm() {
"Should show all suggestions when focusing empty username field",
@[(@"var evt = document.createEvent('Events');"
"username_.focus();"),
@";"],
@""],
@[@"user0 ••••••••", @"abc ••••••••"],
@"[]=, onkeyup=false, onchange=false"
},
{
"Should show password suggestions when focusing password field",
@[(@"var evt = document.createEvent('Events');"
"password_.focus();"),
@";"],
@""],
@[@"user0 ••••••••", @"abc ••••••••"],
@"[]=, onkeyup=false, onchange=false"
},
{
"Should not filter suggestions when focusing username field with input",
@[(@"username_.value='ab';"
"username_.focus();"),
@";"],
@""],
@[@"user0 ••••••••", @"abc ••••••••"],
@"ab[]=, onkeyup=false, onchange=false"
},
Expand All @@ -1132,7 +1132,7 @@ PasswordForm MakeSimpleForm() {
// Keyup event is dispatched to simulate typing
"var ev = new KeyboardEvent('keyup', {bubbles:true});"
"username_.dispatchEvent(ev);"),
@";"],
@""],
@[@"abc ••••••••"],
@"ab[]=, onkeyup=true, onchange=false"
},
Expand All @@ -1144,7 +1144,7 @@ PasswordForm MakeSimpleForm() {
// Keyup event is dispatched to simulate typing.
"var ev = new KeyboardEvent('keyup', {bubbles:true});"
"password_.dispatchEvent(ev);"),
@";"],
@""],
@[],
@"abc[]=••, onkeyup=true, onchange=false"
},
Expand Down Expand Up @@ -1600,15 +1600,15 @@ void SetUp() override {
"Should not show suggest password when focusing username field",
@[(@"var evt = document.createEvent('Events');"
"username_.focus();"),
@";"],
@""],
@[@"user0 ••••••••", @"abc ••••••••"],
@"[]=, onkeyup=false, onchange=false"
},
{
"Should show suggest password when focusing password field",
@[(@"var evt = document.createEvent('Events');"
"password_.focus();"),
@";"],
@""],
@[@"user0 ••••••••", @"abc ••••••••", @"Suggest Password\u2026"],
@"[]=, onkeyup=false, onchange=false"
},
Expand Down
7 changes: 7 additions & 0 deletions ios/chrome/test/earl_grey/chrome_earl_grey.mm
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,9 @@ - (void)loadURL:(const GURL&)URL waitForCompletion:(BOOL)wait {
if (wait) {
[self waitForWebStateVisible];
[self waitForPageToFinishLoading];
EG_TEST_HELPER_ASSERT_TRUE(
[ChromeEarlGreyAppInterface waitForWindowIDInjectionIfNeeded],
@"WindowID failed to inject");
// Loading URL (especially the first time) can trigger alerts.
[SystemAlertHandler handleSystemAlertIfVisible];
}
Expand Down Expand Up @@ -1025,6 +1028,10 @@ - (void)loadURL:(const GURL&)URL
inWindowWithNumber:windowNumber];
if (wait) {
[self waitForPageToFinishLoadingInWindowWithNumber:windowNumber];
EG_TEST_HELPER_ASSERT_TRUE(
[ChromeEarlGreyAppInterface
waitForWindowIDInjectionIfNeededInWindowWithNumber:windowNumber],
@"WindowID failed to inject");
// Loading URL (especially the first time) can trigger alerts.
[SystemAlertHandler handleSystemAlertIfVisible];
}
Expand Down
10 changes: 10 additions & 0 deletions ios/chrome/test/earl_grey/chrome_earl_grey_app_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@
// ui::PAGE_TRANSITION_TYPED and returns without waiting for the page to load.
+ (void)startLoadingURL:(NSString*)spec;

// If the current WebState is HTML content, will wait until the window ID is
// injected. Returns YES if the injection is successful or if the WebState is
// not HTML content.
+ (BOOL)waitForWindowIDInjectionIfNeeded;

// Returns YES if the current WebState is loading.
+ (BOOL)isLoading;

Expand Down Expand Up @@ -210,6 +215,11 @@
// Returns YES if the current WebState in window with given number is loading.
+ (BOOL)isLoadingInWindowWithNumber:(int)windowNumber [[nodiscard]];

// If the current WebState in window with given number is HTML content, will
// wait until the window ID is injected. Returns YES if the injection is
// successful or if the WebState is not HTML content.
+ (BOOL)waitForWindowIDInjectionIfNeededInWindowWithNumber:(int)windowNumber;

// Returns YES if the current WebState in window with given number contains
// `text`.
+ (BOOL)webStateContainsText:(NSString*)text
Expand Down
22 changes: 22 additions & 0 deletions ios/chrome/test/earl_grey/chrome_earl_grey_app_interface.mm
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
#import "ios/web/public/js_messaging/web_frame.h"
#import "ios/web/public/js_messaging/web_frame_util.h"
#import "ios/web/public/navigation/navigation_manager.h"
#import "ios/web/public/test/earl_grey/js_test_util.h"
#import "ios/web/public/test/element_selector.h"
#import "ios/web/public/test/url_test_util.h"
#import "ios/web/public/test/web_view_content_test_util.h"
Expand Down Expand Up @@ -193,6 +194,16 @@ + (void)startLoadingURL:(NSString*)spec {
chrome_test_util::LoadUrl(GURL(base::SysNSStringToUTF8(spec)));
}

+ (BOOL)waitForWindowIDInjectionIfNeeded {
web::WebState* webState = chrome_test_util::GetCurrentWebState();

if (webState->ContentIsHTML()) {
return web::WaitUntilWindowIdInjected(webState);
}

return YES;
}

+ (bool)isLoading {
return chrome_test_util::IsLoading();
}
Expand Down Expand Up @@ -496,6 +507,17 @@ + (BOOL)isLoadingInWindowWithNumber:(int)windowNumber {
return chrome_test_util::IsLoadingInWindowWithNumber(windowNumber);
}

+ (BOOL)waitForWindowIDInjectionIfNeededInWindowWithNumber:(int)windowNumber {
web::WebState* webState =
chrome_test_util::GetCurrentWebStateForWindowWithNumber(windowNumber);

if (webState->ContentIsHTML()) {
return web::WaitUntilWindowIdInjected(webState);
}

return YES;
}

+ (BOOL)webStateContainsText:(NSString*)text
inWindowWithNumber:(int)windowNumber {
web::WebState* webState =
Expand Down
8 changes: 8 additions & 0 deletions ios/web/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ source_set("web") {
deps = [
":common_js",
":core",
":js_resources",
":message_js",
":navigation_resources",
":plugin_placeholder_js",
Expand Down Expand Up @@ -117,6 +118,8 @@ source_set("eg_app_support+eg2") {
]

sources = [
"public/test/earl_grey/js_test_util.h",
"public/test/earl_grey/js_test_util.mm",
"public/test/earl_grey/web_view_actions.h",
"public/test/earl_grey/web_view_actions.mm",
"public/test/earl_grey/web_view_matchers.h",
Expand Down Expand Up @@ -663,6 +666,11 @@ optimize_js("message_js") {
sources = [ "js_messaging/resources/message.js" ]
}

optimize_js("js_resources") {
primary_script = "js_messaging/resources/window_id.js"
sources = [ "js_messaging/resources/window_id.js" ]
}

bundle_data("navigation_resources") {
sources = [
"navigation/resources/error_page_injected.html",
Expand Down
3 changes: 3 additions & 0 deletions ios/web/js_messaging/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ source_set("js_messaging") {
]

sources = [
"crw_js_window_id_manager.h",
"crw_js_window_id_manager.mm",
"page_script_util.h",
"page_script_util.mm",
"web_frame_impl.h",
Expand Down Expand Up @@ -179,6 +181,7 @@ source_set("unittests") {
]

sources = [
"crw_js_window_id_manager_unittest.mm",
"java_script_content_world_unittest.mm",
"java_script_feature_manager_unittest.mm",
"java_script_feature_unittest.mm",
Expand Down
29 changes: 29 additions & 0 deletions ios/web/js_messaging/crw_js_window_id_manager.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2014 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef IOS_WEB_JS_MESSAGING_CRW_JS_WINDOW_ID_MANAGER_H_
#define IOS_WEB_JS_MESSAGING_CRW_JS_WINDOW_ID_MANAGER_H_

#import <Foundation/Foundation.h>
#import <WebKit/WebKit.h>

// Injects the JavaScript file window_id.js which sets __gCrWeb.windowId and
// manages the windowId for Page->Native->Page messages.
@interface CRWJSWindowIDManager : NSObject

// A unique window ID is assigned when the script is injected. Can not be null.
@property(nonatomic, copy, readonly) NSString* windowID;

- (instancetype)init NS_UNAVAILABLE;

// Initializes CRWJSWindowIDManager. `webView` will be used for script
// evaluation to inject window ID and can not be null.
- (instancetype)initWithWebView:(WKWebView*)webView NS_DESIGNATED_INITIALIZER;

// Injects windowId to a web page.
- (void)inject;

@end

#endif // IOS_WEB_JS_MESSAGING_CRW_JS_WINDOW_ID_MANAGER_H_
125 changes: 125 additions & 0 deletions ios/web/js_messaging/crw_js_window_id_manager.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// Copyright 2014 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#import "ios/web/js_messaging/crw_js_window_id_manager.h"

#import <ostream>

#import "base/dcheck_is_on.h"
#import "base/logging.h"
#import "base/metrics/histogram_macros.h"
#import "base/notreached.h"
#import "base/strings/string_number_conversions.h"
#import "base/strings/sys_string_conversions.h"
#import "base/time/time.h"
#import "crypto/random.h"
#import "ios/web/js_messaging/page_script_util.h"

#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif

namespace {
// Number of random bytes in unique key for window ID. The length of the
// window ID will be twice this number, as it is hexadecimal encoded.
const size_t kUniqueKeyLength = 16;

#if DCHECK_IS_ON()
// The time in seconds which is determined to be a long wait for the injection
// of the window ID. The wait time will be logged if the time exceeds this
// value.
const double kSignificantInjectionTime = 0.1;
#endif

} // namespace

@interface CRWJSWindowIDManager () {
// Web view used for script evaluation to inject window ID.
WKWebView* _webView;
// Backs up property with the same name.
NSString* _windowID;
}

// Returns a string of randomized ASCII characters.
+ (NSString*)newUniqueKey;

@end

@implementation CRWJSWindowIDManager

- (NSString*)windowID {
return _windowID;
}

- (instancetype)initWithWebView:(WKWebView*)webView {
if ((self = [super init])) {
_webView = webView;
_windowID = [[self class] newUniqueKey];
}
return self;
}

- (void)inject {
[self injectWithStartTime:base::TimeTicks::Now()];
}

- (void)injectWithStartTime:(base::TimeTicks)startTime {
_windowID = [[self class] newUniqueKey];
NSString* script = [web::GetPageScript(@"window_id")
stringByReplacingOccurrencesOfString:@"$(WINDOW_ID)"
withString:_windowID];
// WKUserScript for message API may not be injected yet. Make windowID script
// return boolean indicating whether the injection was successful.
NSString* scriptWithResult = [NSString
stringWithFormat:@"if (!window.__gCrWeb || !window.__gCrWeb.message) "
@"{false; } else { %@; true; }",
script];

__weak CRWJSWindowIDManager* weakSelf = self;
[_webView evaluateJavaScript:scriptWithResult
completionHandler:^(id result, NSError* error) {
CRWJSWindowIDManager* strongSelf = weakSelf;
if (!strongSelf)
return;
if (error) {
return;
}

// If `result` is an incorrect type, do not check its value.
// Also do not attempt to re-inject scripts as it may lead to
// endless recursion attempting to inject the scripts correctly.
if (result && CFBooleanGetTypeID() !=
CFGetTypeID((__bridge CFTypeRef)result)) {
NOTREACHED();
return;
}

if (![result boolValue]) {
// WKUserScript has not been injected yet. Retry window id
// injection, because it is critical for the system to
// function.
[strongSelf injectWithStartTime:startTime];
} else {
base::TimeDelta elapsed = base::TimeTicks::Now() - startTime;
#if DCHECK_IS_ON()
DLOG_IF(WARNING,
elapsed.InSecondsF() > kSignificantInjectionTime)
<< "Elapsed time for windowID injection: " << elapsed;
#endif
UMA_HISTOGRAM_TIMES("IOS.WindowIDInjection.ElapsedTime",
elapsed);
}
}];
}

#pragma mark - Private

+ (NSString*)newUniqueKey {
char randomBytes[kUniqueKeyLength];
crypto::RandBytes(randomBytes, kUniqueKeyLength);
std::string result = base::HexEncode(randomBytes, kUniqueKeyLength);
return base::SysUTF8ToNSString(result);
}

@end

0 comments on commit f860c14

Please sign in to comment.