New issue
Advanced search Search tips

Issue 697621 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Compile failure on Windows_beta Official desktop builder

Project Member Reported by pbomm...@chromium.org, Mar 1 2017

Issue description

Link to Builder:
================
https://uberchromegw.corp.google.com/i/official.desktop.continuous/builders/win%20beta/builds/6624

Link to Log File:
=================
https://uberchromegw.corp.google.com/i/official.desktop.continuous/builders/win%20beta/builds/6624/steps/compile/logs/stdio

Error from the log:
====================
FAILED: obj/chrome/browser/browser_4/render_view_context_menu.obj 
ninja -t msvc -e environment.x64 -- "C:\b\depot_tools\win_toolchain\vs_files\d3cb0e37bdd120ad0ac4650b674b09e81be45616\VC\bin\amd64/cl.exe" /nologo /showIncludes /FC @obj/chrome/browser/browser_4/render_view_context_menu.obj.rsp /c ../../chrome/browser/renderer_context_menu/render_view_context_menu.cc /Foobj/chrome/browser/browser_4/render_view_context_menu.obj /Fd"obj/chrome/browser/browser_4_cc.pdb"
c:\b\build\slave\win_beta\build\src\chrome\browser\renderer_context_menu\render_view_context_menu.cc(865): error C2601: 'RenderViewContextMenu::GetProfile': local function definitions are illegal
c:\b\build\slave\win_beta\build\src\chrome\browser\renderer_context_menu\render_view_context_menu.cc(746): note: this line contains a '{' which has not yet been matched
c:\b\build\slave\win_beta\build\src\chrome\browser\renderer_context_menu\render_view_context_menu.cc(869): error C2601: 'RenderViewContextMenu::RecordUsedItem': local function definitions are illegal
c:\b\build\slave\win_beta\build\src\chrome\browser\renderer_context_menu\render_view_context_menu.cc(746): note: this line contains a '{' which has not yet been matched
c:\b\build\slave\win_beta\build\src\chrome\browser\renderer_context_menu\render_view_context_menu.cc(905): error C2601: 'RenderViewContextMenu::RecordShownItem': local function definitions are illegal
c:\b\build\slave\win_beta\build\src\chrome\browser\renderer_context_menu\render_view_context_menu.cc(746): note: this line contains a '{' which has not yet been matched
 
Pls revert of fix this ASAP. Thank you.
The merge seems to have gone wrong for some reason:

diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu.cc b/chrome/browser/renderer_context_menu/render_view_context_menu.cc
index 92d96ddcd3a8..ec87327d4e57 100644
--- a/chrome/browser/renderer_context_menu/render_view_context_menu.cc
+++ b/chrome/browser/renderer_context_menu/render_view_context_menu.cc
@@ -853,6 +853,12 @@ void RenderViewContextMenu::InitMenu() {
   if (content_type_->SupportsGroup(
           ContextMenuContentType::ITEM_GROUP_PASSWORD)) {
     AppendPasswordItems();
+
+  // Remove any redundant trailing separator.
+  if (menu_model_.GetItemCount() > 0 &&
+      menu_model_.GetTypeAt(menu_model_.GetItemCount() - 1) ==
+          ui::MenuModel::TYPE_SEPARATOR) {
+    menu_model_.RemoveItemAt(menu_model_.GetItemCount() - 1);
   }
 }

I'm hoping I can manually fix the curly brace issue.
@lazyboy, thanks for looking at this. Missed there was a missing brace.

Let me know if you need me to do anything.
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 1 2017

Labels: merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/85108a060f1fa35ab6f0eb6f45827bc881937f66

commit 85108a060f1fa35ab6f0eb6f45827bc881937f66
Author: Istiaque Ahmed <lazyboy@chromium.org>
Date: Wed Mar 01 22:54:19 2017

Revert "Remove redundant separator at end of the spelling correction context menu"

The merge was incorrect, breaking compile. Will re-merge soon.

This reverts commit dc2bb753a4f87323a7d6473a6421d16f2e3da769.

BUG= 697621 ,  695800 
TBR=lazyboy@chromium.org

Review-Url: https://codereview.chromium.org/2722143003 .
Cr-Commit-Position: refs/branch-heads/2987@{#733}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/85108a060f1fa35ab6f0eb6f45827bc881937f66/chrome/browser/renderer_context_menu/render_view_context_menu.cc

@edwardjung, seemed easier to revert the merge first ^^^
Will re-merge the original CL soon.
Status: Fixed (was: Assigned)
Re-merge was done. https://codereview.chromium.org/2724973002
Everything should go green.

Sign in to add a comment