New issue
Advanced search Search tips

Issue 758098 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Compile failure on ios-{device, simulator}-xcode-clang

Project Member Reported by vitaliii@chromium.org, Aug 23 2017

Issue description

In both cases the error is

FAILED: obj/ios/web/web/crw_context_menu_controller.o 
export DEVELOPER_DIR=/b/build/slave/ios-device-xcode-clang/build/src/build/ios_files/Xcode.app;  /b/build/slave/cache/goma_client/gomacc clang++ -MMD -MF obj/ios/web/web/crw_context_menu_controller.o.d -DV8_DEPRECATION_WARNINGS -DNO_TCMALLOC -DDISABLE_NACL -DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=\"310694-2\" -DCR_XCODE_VERSION=0821 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DNS_BLOCK_ASSERTIONS=1 -DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DHAVE_PTHREAD -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DUCHAR_TYPE=uint16_t -DI18N_ADDRESSINPUT_USE_BASICTYPES_OVERRIDE=1 -DI18N_ADDRESS_VALIDATION_DATA_URL=\"https://chromium-i18n.appspot.com/ssl-aggregate-address/\" -DSK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS -DSK_HAS_PNG_LIBRARY -DSK_HAS_WEBP_LIBRARY -DSK_SUPPORT_GPU=0 -I../.. -Igen -I../../third_party/libwebp/src -I../../third_party/protobuf/src -Igen/protoc_out -I../../third_party/protobuf/src -I../../third_party/boringssl/src/include -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -I../../third_party/libaddressinput/src/cpp/include -I../../third_party/libaddressinput/chromium/override -I../../third_party/ced/src -I../../skia/config -I../../skia/ext -I../../third_party/skia/include/c -I../../third_party/skia/include/config -I../../third_party/skia/include/core -I../../third_party/skia/include/effects -I../../third_party/skia/include/encode -I../../third_party/skia/include/gpu -I../../third_party/skia/include/images -I../../third_party/skia/include/lazy -I../../third_party/skia/include/pathops -I../../third_party/skia/include/pdf -I../../third_party/skia/include/pipe -I../../third_party/skia/include/ports -I../../third_party/skia/include/utils -I../../third_party/skia/third_party/vulkan -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -fcolor-diagnostics -arch armv7 -Wall -Werror -Wextra -Wundeclared-selector -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Oz -fno-omit-frame-pointer -gdwarf-2 -isysroot /b/build/slave/ios-device-xcode-clang/build/src/build/ios_files/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS10.2.sdk -stdlib=libc++ -miphoneos-version-min=9.0 -fvisibility=hidden -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -std=c++14 -fobjc-call-cxx-cdtors -Wobjc-missing-property-synthesis -fno-rtti -fno-exceptions -fvisibility-inlines-hidden -fobjc-arc -c ../../ios/web/web_state/ui/crw_context_menu_controller.mm -o obj/ios/web/web/crw_context_menu_controller.o
../../ios/web/web_state/ui/crw_context_menu_controller.mm:233:18: error: using integer absolute value function 'abs' when argument is of floating point type [-Werror,-Wabsolute-value]
  float deltaX = abs(_locationForLastTouch.x - currentTouchLocation.x);
                 ^
../../ios/web/web_state/ui/crw_context_menu_controller.mm:233:18: note: use function 'std::abs' instead
  float deltaX = abs(_locationForLastTouch.x - currentTouchLocation.x);
                 ^~~
                 std::abs
../../ios/web/web_state/ui/crw_context_menu_controller.mm:234:18: error: using integer absolute value function 'abs' when argument is of floating point type [-Werror,-Wabsolute-value]
  float deltaY = abs(_locationForLastTouch.y - currentTouchLocation.y);
                 ^
../../ios/web/web_state/ui/crw_context_menu_controller.mm:234:18: note: use function 'std::abs' instead
  float deltaY = abs(_locationForLastTouch.y - currentTouchLocation.y);
                 ^~~
                 std::abs
2 errors generated.
The failing code was added in

https://chromium-review.googlesource.com/c/chromium/src/+/579585

There is a revert created, but it did not land

https://chromium-review.googlesource.com/c/chromium/src/+/627964
Cc: michaeldo@chromium.org
I am reverting the CL mentioned in comment 2.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 23 2017

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

commit 735aa36217a394455f70aad87e2c844053fd810a
Author: vitaliii <vitaliii@chromium.org>
Date: Wed Aug 23 08:16:21 2017

Revert "Always suppress system WKWebView Context Menu and show custom context menu."

This reverts commit 2d85a12f776d580fd9d2f12da4a8a94ee81d3245.

Reason for revert: compile error on ios trybots, see  crbug.com/758098 .

Original change's description:
> Always suppress system WKWebView Context Menu and show custom context menu.
> 
> Triggering the context menu is done independently of fetching details of the DOM
> element the user long pressed. Previously, if the details of the DOM element
> were not available when the long press recognizer completed, the system context
> menu would be displayed instead of Chrome's custom context menu.
> 
> The system context menu can cause the user lots of confusion. For example, the
> "Add to Reading List" item which is shown on the system context menu conflicts
> with the Chrome Reading List feature. If the user taps the system Reading List
> item, the item will not be added to their Chrome Reading list, but they will
> likely believe that it has been successfully added.
> 
> To resolve these problems, this change always suppresses the system context
> menu and waits for the DOM element to be returned before showing the custom
> Chrome Context Menu. In addition, iFrames will no longer show the WKWebView
> context menu.
> 
> Now, the custom context menu will always be displayed once the DOM element
> details have been fetched as long as the user is still long pressing on the
> element.
> 
> Bug: 542933
> Change-Id: Ia47e659868449bb514fc6b679e0f233c2bcd8182
> Reviewed-on: https://chromium-review.googlesource.com/579585
> Reviewed-by: Eugene But <eugenebut@chromium.org>
> Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#496606}

TBR=eugenebut@chromium.org,michaeldo@chromium.org

Change-Id: If09148dc42aa8733a4cdb33278c289fffdd973ee
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 542933, 758098 
Reviewed-on: https://chromium-review.googlesource.com/628096
Reviewed-by: vitaliii <vitaliii@chromium.org>
Commit-Queue: vitaliii <vitaliii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496623}
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/chrome/browser/ui/static_content/static_html_view_controller.mm
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/clean/chrome/browser/ui/web_contents/web_coordinator.mm
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web/public/test/crw_mock_web_state_delegate.mm
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web/public/test/fakes/test_web_state_delegate.h
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web/public/test/fakes/test_web_state_delegate.mm
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web/public/web_state/ui/crw_context_menu_delegate.h
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web/public/web_state/ui/crw_native_content.h
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web/public/web_state/web_state_delegate.h
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web/public/web_state/web_state_delegate_bridge.h
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web/shell/view_controller.mm
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web/web_state/ui/crw_context_menu_controller.mm
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web/web_state/web_state_delegate.mm
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web/web_state/web_state_delegate_bridge.mm
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web/web_state/web_state_impl.h
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web/web_state/web_state_impl.mm
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web_view/internal/cwv_web_view.mm
[modify] https://crrev.com/735aa36217a394455f70aad87e2c844053fd810a/ios/web_view/public/cwv_ui_delegate.h

The revert has landed.

Funny enough the revert does not land automatically, one needs to self LGTM it for TBRs to work. It looks like that's why the previous revert did not land, I've notified its owner.

Sign in to add a comment