Skip to content

Commit

Permalink
Revert "[WebLayer] Show security icon."
Browse files Browse the repository at this point in the history
This reverts commit c5743c6.

Reason for revert: Broke ChromeBundleSmokeTest.*, see crbug.com/1050637

Original change's description:
> [WebLayer] Show security icon.
> 
> UrlBarView is now a LinearLayout with its own resource XML. It contains
> a security icon (currently always the lock icon) and a TextView
> containing current display URL.
> 
> Here's a recording of the UI after this change:
> https://drive.google.com/a/google.com/file/d/1q8IzYALoPYd8KLcln4cMEnRjIT6ox_so/view?usp=sharing
> 
> Bug: 1025607
> Change-Id: I5a2a364a3bf8f7f7e26adf2c8a61683785044aa0
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2039672
> Commit-Queue: Mugdha Lakhani <nator@chromium.org>
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Reviewed-by: Richard Coles <torne@chromium.org>
> Reviewed-by: Colin Blundell <blundell@chromium.org>
> Reviewed-by: Tommy Li <tommycli@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#739850}

TBR=dtrainor@chromium.org,torne@chromium.org,tommycli@chromium.org,blundell@chromium.org,nator@chromium.org

Change-Id: I3b5320b8214e9bda3918dcc30b45402171e3ec4f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1025607
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2047280
Reviewed-by: Sky Malice <skym@chromium.org>
Commit-Queue: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739954}
  • Loading branch information
Sky Malice authored and Commit Bot committed Feb 10, 2020
1 parent cab8829 commit a880066
Show file tree
Hide file tree
Showing 29 changed files with 39 additions and 189 deletions.
3 changes: 1 addition & 2 deletions chrome/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ android_resources("chrome_app_java_resources") {
"//components/browser_ui/widget/android:java_resources",
"//components/find_in_page/android:java_resources",
"//components/javascript_dialogs/android:java_resources",
"//components/omnibox/browser:java_resources",
"//components/policy:app_restrictions_resources",
"//components/strings:components_locale_settings_grd",
"//components/strings:components_strings_grd",
Expand Down Expand Up @@ -430,6 +429,7 @@ android_library("chrome_java") {
"//components/payments/content/android:error_strings_generated_srcjar",
"//components/payments/content/android:method_strings_generated_srcjar",
"//components/search_engines:search_engine_type_java",
"//components/security_state/core:security_state_enums_java",
"//components/signin/core/browser:signin_enums_javagen",
"//components/ui_metrics:ui_metrics_enums_java",
"//content/public/browser:contacts_picker_properties_requested_javagen",
Expand Down Expand Up @@ -2872,7 +2872,6 @@ android_library("native_java_unittests_java") {
"//chrome/browser/flags:java",
"//chrome/browser/util:java",
"//chrome/test/android:chrome_java_test_support",
"//components/omnibox/browser:browser_java",
"//components/payments/content/android:java",
"//content/public/android:content_java",
"//content/public/test/android:android_test_message_pump_support_java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import org.chromium.chrome.browser.util.UrlUtilities;
import org.chromium.components.browser_ui.styles.ChromeColors;
import org.chromium.components.dom_distiller.core.DomDistillerUrlUtils;
import org.chromium.components.omnibox.SecurityStatusIcon;
import org.chromium.components.security_state.ConnectionSecurityLevel;
import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.util.ColorUtils;
Expand Down Expand Up @@ -412,20 +411,31 @@ int getSecurityIconResource(
return R.drawable.ic_offline_pin_24dp;
}

// Return early if native initialization hasn't been done yet.
if ((securityLevel == ConnectionSecurityLevel.NONE
|| securityLevel == ConnectionSecurityLevel.WARNING)
&& mNativeLocationBarModelAndroid == 0) {
return R.drawable.omnibox_info;
switch (securityLevel) {
case ConnectionSecurityLevel.NONE:
return isSmallDevice
&& (!SearchEngineLogoUtils.shouldShowSearchEngineLogo(isIncognito())
|| getNewTabPageForCurrentTab() != null)
? 0
: R.drawable.omnibox_info;
case ConnectionSecurityLevel.WARNING:
if (mNativeLocationBarModelAndroid == 0) {
return R.drawable.omnibox_info;
}
if (SecurityStateModel.shouldShowDangerTriangleForWarningLevel()) {
return R.drawable.omnibox_not_secure_warning;
}
return R.drawable.omnibox_info;
case ConnectionSecurityLevel.DANGEROUS:
return R.drawable.omnibox_not_secure_warning;
case ConnectionSecurityLevel.SECURE_WITH_POLICY_INSTALLED_CERT:
case ConnectionSecurityLevel.SECURE:
case ConnectionSecurityLevel.EV_SECURE:
return R.drawable.omnibox_https_valid;
default:
assert false;
}

boolean skipIconForNeutralState =
!SearchEngineLogoUtils.shouldShowSearchEngineLogo(isIncognito())
|| getNewTabPageForCurrentTab() != null;

return SecurityStatusIcon.getSecurityIconResource(securityLevel,
SecurityStateModel.shouldShowDangerTriangleForWarningLevel(), isSmallDevice,
skipIconForNeutralState);
return 0;
}

@Override
Expand Down
12 changes: 1 addition & 11 deletions components/omnibox/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -269,27 +269,17 @@ jumbo_static_library("browser") {
}

if (is_android) {
android_resources("java_resources") {
resource_dirs = [ "android/java/res" ]
custom_package = "org.chromium.components.omnibox"
}

android_library("browser_java") {
sources = [
"android/java/src/org/chromium/components/omnibox/SecurityStatusIcon.java",
"android/java/src/org/chromium/components/omnibox/SuggestionAnswer.java",
]

deps = [
":java_resources",
"//base:base_java",
"//third_party/android_deps:com_android_support_support_compat_java",
]

srcjar_deps = [
":browser_java_enums_srcjar",
"//components/security_state/core:security_state_enums_java",
]
srcjar_deps = [ ":browser_java_enums_srcjar" ]
}

java_cpp_enum("browser_java_enums_srcjar") {
Expand Down

This file was deleted.

1 change: 0 additions & 1 deletion weblayer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,6 @@ jumbo_static_library("weblayer_lib") {
"//components/security_interstitials/content:security_interstitial_page",
"//components/security_interstitials/content/renderer:security_interstitial_page_controller",
"//components/security_interstitials/core",
"//components/security_state/content",
"//components/sessions",
"//components/spellcheck:buildflags",
"//components/ssl_errors",
Expand Down
1 change: 0 additions & 1 deletion weblayer/DEPS
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
include_rules = [
"+components/omnibox/browser",
"+components/security_state/core/security_state.h",
"+third_party/metrics_proto/omnibox_input_type.pb.h",
]
2 changes: 0 additions & 2 deletions weblayer/browser/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ include_rules = [
"+components/safe_browsing/core/common",
"+components/safe_browsing/core/features.h",
"+components/security_interstitials",
"+components/security_state/content/content_utils.h",
"+components/security_state/core/security_state.h",
"+components/sessions",
"+components/spellcheck/browser",
"+components/ssl_errors",
Expand Down
4 changes: 1 addition & 3 deletions weblayer/browser/java/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import("//build/config/android/rules.gni")
import("//weblayer/variables.gni")

android_resources("weblayer_resources") {
resource_dirs = [ "res" ]
resource_dirs = []
custom_package = "org.chromium.weblayer_private"
}

Expand Down Expand Up @@ -78,15 +78,13 @@ android_library("java") {
"//components/find_in_page/android:java",
"//components/metrics:metrics_java",
"//components/minidump_uploader:minidump_uploader_java",
"//components/omnibox/browser:browser_java",
"//components/security_interstitials/content/android:java",
"//components/spellcheck/browser/android:java",
"//components/variations/android:variations_java",
"//components/version_info/android:version_constants_java",
"//content/public/android:content_java",
"//net/android:net_java",
"//third_party/android_deps:com_android_support_support_compat_java",
"//ui/android:ui_full_java",
"//ui/android:ui_java",
]
srcjar_deps = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.ui.base.DeviceFormFactor;
import org.chromium.ui.base.WindowAndroid;
import org.chromium.weblayer_private.interfaces.APICallException;
import org.chromium.weblayer_private.interfaces.IBrowser;
Expand Down Expand Up @@ -206,11 +205,6 @@ public Context getContext() {
return mWindowAndroid.getContext().get();
}

public boolean isWindowOnSmallDevice() {
assert mWindowAndroid != null;
return !DeviceFormFactor.isWindowOnTablet(mWindowAndroid);
}

@Override
public IProfile getProfile() {
StrictModeWorkaround.apply();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,11 @@
import android.os.Bundle;
import android.support.annotation.NonNull;
import android.util.TypedValue;
import android.view.View;
import android.widget.ImageButton;
import android.widget.LinearLayout;
import android.widget.TextView;

import androidx.annotation.DrawableRes;

import org.chromium.base.LifetimeAssert;
import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.components.omnibox.SecurityStatusIcon;
import org.chromium.weblayer_private.interfaces.IObjectWrapper;
import org.chromium.weblayer_private.interfaces.IUrlBarController;
import org.chromium.weblayer_private.interfaces.ObjectWrapper;
Expand Down Expand Up @@ -72,19 +66,15 @@ public UrlBarControllerImpl(BrowserImpl browserImpl, long nativeBrowser) {
}

protected class UrlBarView
extends LinearLayout implements BrowserImpl.VisibleSecurityStateObserver {
extends TextView implements BrowserImpl.VisibleSecurityStateObserver {
private float mTextSize;
private TextView mUrlTextView;
private ImageButton mSecurityButton;

public UrlBarView(@NonNull Context context, Bundle options) {
super(context);
mTextSize = options.getFloat(URL_TEXT_SIZE, DEFAULT_TEXT_SIZE);
View.inflate(getContext(), R.layout.url_bar, this);
mUrlTextView = findViewById(R.id.url_text);
mSecurityButton = (ImageButton) findViewById(R.id.security_button);

updateView();

mTextSize = options.getFloat(URL_TEXT_SIZE, DEFAULT_TEXT_SIZE);
setTextSize(TypedValue.COMPLEX_UNIT_SP, Math.max(MINIMUM_TEXT_SIZE, mTextSize));
}

// BrowserImpl.VisibleSecurityStateObserver
Expand All @@ -110,28 +100,12 @@ protected void onDetachedFromWindow() {
}

private void updateView() {
// TODO(crbug.com/1025607): Add a way to get a formatted URL based
// on mOptions.
if (mBrowserImpl == null) return;
String displayUrl =
UrlBarControllerImplJni.get().getUrlForDisplay(mNativeUrlBarController);
mUrlTextView.setText(displayUrl);
mUrlTextView.setTextSize(
TypedValue.COMPLEX_UNIT_SP, Math.max(MINIMUM_TEXT_SIZE, mTextSize));

mSecurityButton.setImageResource(getSecurityIcon());

// TODO(crbug.com/1025607): Set content description for accessibility, and a click
// listener.
}

@DrawableRes
private int getSecurityIcon() {
return SecurityStatusIcon.getSecurityIconResource(
UrlBarControllerImplJni.get().getConnectionSecurityLevel(
mNativeUrlBarController),
UrlBarControllerImplJni.get().shouldShowDangerTriangleForWarningLevel(
mNativeUrlBarController),
mBrowserImpl.isWindowOnSmallDevice(),
/* skipIconForNeutralState= */ true);
setText(displayUrl);
}
}

Expand All @@ -140,7 +114,5 @@ interface Natives {
long createUrlBarController(long browserPtr);
void deleteUrlBarController(long urlBarControllerImplPtr);
String getUrlForDisplay(long nativeUrlBarControllerImpl);
int getConnectionSecurityLevel(long nativeUrlBarControllerImpl);
boolean shouldShowDangerTriangleForWarningLevel(long nativeUrlBarControllerImpl);
}
}
30 changes: 0 additions & 30 deletions weblayer/browser/java/res/layout/url_bar.xml

This file was deleted.

41 changes: 5 additions & 36 deletions weblayer/browser/urlbar/url_bar_controller_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
#include "base/strings/utf_string_conversions.h"
#include "components/omnibox/browser/autocomplete_input.h"
#include "components/omnibox/browser/location_bar_model_impl.h"
#include "components/security_state/content/content_utils.h"
#include "components/security_state/core/security_state.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_constants.h"
#include "weblayer/browser/browser_impl.h"
Expand Down Expand Up @@ -61,39 +59,18 @@ UrlBarControllerImpl::GetUrlForDisplay(JNIEnv* env) {
return base::android::ScopedJavaLocalRef<jstring>(
base::android::ConvertUTF16ToJavaString(env, GetUrlForDisplay()));
}

jint UrlBarControllerImpl::GetConnectionSecurityLevel(JNIEnv* env) {
return GetConnectionSecurityLevel();
}

jboolean UrlBarControllerImpl::ShouldShowDangerTriangleForWarningLevel(
JNIEnv* env) {
return ShouldShowDangerTriangleForWarningLevel();
}
#endif

base::string16 UrlBarControllerImpl::GetUrlForDisplay() {
return location_bar_model_->GetURLForDisplay();
}

security_state::SecurityLevel
UrlBarControllerImpl::GetConnectionSecurityLevel() {
auto* active_web_contents = GetActiveWebContents();
if (!active_web_contents)
return security_state::SecurityLevel::NONE;

auto state = security_state::GetVisibleSecurityState(active_web_contents);
DCHECK(state);
return security_state::GetSecurityLevel(
*state, /* used_policy_installed_certificate= */ false);
}

bool UrlBarControllerImpl::ShouldShowDangerTriangleForWarningLevel() {
return security_state::ShouldShowDangerTriangleForWarningLevel();
}

bool UrlBarControllerImpl::GetURL(GURL* url) const {
auto* active_web_contents = GetActiveWebContents();
auto* active_tab = static_cast<TabImpl*>(browser_->GetActiveTab());
if (!active_tab)
return false;

auto* active_web_contents = active_tab->web_contents();
if (!active_web_contents)
return false;

Expand All @@ -113,12 +90,4 @@ base::string16 UrlBarControllerImpl::FormattedStringWithEquivalentMeaning(
url, formatted_url, AutocompleteSchemeClassifierImpl(), nullptr);
}

content::WebContents* UrlBarControllerImpl::GetActiveWebContents() const {
auto* active_tab = static_cast<TabImpl*>(browser_->GetActiveTab());
if (!active_tab)
return nullptr;

return active_tab->web_contents();
}

} // namespace weblayer
Loading

0 comments on commit a880066

Please sign in to comment.