New issue
Advanced search Search tips

Issue 921784 link

Starred by 1 user
Status: Started
Owner:
laisminchillo@chromium.org
Cc:
ntfschr@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

DEV_SUFFIX shouldn't be applied in the boundary interface.

Project Member Reported by tobiasjs@chromium.org, Jan 14 
Currently the proxy override feature string declared in the boundary interface is PROXY_OVERRIDE = "PROXY_OVERRIDE:3" + DEV_SUFFIX;

This doesn't work as intended, because what the feature detection code does is check for the export of $FEATURE and $FEATURE + :dev - and if $FEATURE already has :dev it's looking for a double suffix.

The right place to add this is in SupportLibWebViewChromiumFactory

For now, we should correct this instance, but for future the documentation should be improved and ideally there should be a presubmit that enforces it.

Comment 1 by ntfschr@chromium.org, Jan 15 

Could probably enforce this in robolectric instead of presubmit. Just iterate over every declared feature and fail the test if any strings end in ":dev". I'd much prefer a test to a presubmit check.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 17 (5 days ago) 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b01b84ca1badeed8ff5d8c034011515f697a157c

commit b01b84ca1badeed8ff5d8c034011515f697a157c
Author: Laís Minchillo <laisminchillo@chromium.org>
Date: Thu Jan 17 11:18:06 2019

[aw] Add presubmit check for DEV_SUFFIX

DEV_SUFFIX shouldn't be applied in the boundary interface. The right
place to add this is in SupportLibWebViewChromiumFactory.

Bug: 921784
Change-Id: I62cddca5f4f5d817c4f25baf60f45fcd31b240e5
Reviewed-on: https://chromium-review.googlesource.com/c/1412464
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Commit-Queue: Laís Minchillo <laisminchillo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623644}
[modify] https://crrev.com/b01b84ca1badeed8ff5d8c034011515f697a157c/android_webview/support_library/PRESUBMIT.py

Comment 3 by laisminchillo@chromium.org, Jan 17 (5 days ago)

Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Yesterday (38 hours ago) 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/93831f2301c40d3e93218d350d3538a00fef2de0

commit 93831f2301c40d3e93218d350d3538a00fef2de0
Author: Laís Minchillo <laisminchillo@chromium.org>
Date: Mon Jan 21 16:20:40 2019

[aw] Remove DEV_SUFFIX from boundary interface

Removes DEV_SUFFIX from PROXY_OVERRIDE in boundary interface.

Bug: 921784
Change-Id: I074a7396edc3649aed22f7f77e47c1d1094ffef2
Reviewed-on: https://chromium-review.googlesource.com/c/1411878
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Commit-Queue: Laís Minchillo <laisminchillo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624599}
[modify] https://crrev.com/93831f2301c40d3e93218d350d3538a00fef2de0/android_webview/support_library/boundary_interfaces/src/org/chromium/support_lib_boundary/util/Features.java
[modify] https://crrev.com/93831f2301c40d3e93218d350d3538a00fef2de0/android_webview/support_library/java/src/org/chromium/support_lib_glue/SupportLibWebViewChromiumFactory.java
►

Sign in to add a comment