New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 765829 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Refactor repetitive use of range check of custom extension menu ids

Project Member Reported by lazyboy@chromium.org, Sep 15 2017

Issue description

While reviewing CLs, I've noticed that we have repeating pattern (at least in 5 places in production code) of checking whether a menu item id is an extension custom id or not:

  if (command_id >= IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST &&
      command_id <= IDC_EXTENSIONS_CONTEXT_CUSTOM_LAST) ...

ContextMenuMatcher::IsExtensionsCustomCommandId() is probably what we want them to be replace with (I haven't confirmed).

Catherine, can you take a quick look? Should be an easy one if you have some free cycles.
 
Status: Started (was: Assigned)
Status: Fixed (was: Started)
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 2 2017

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

commit 8e7c1213f30e0f24539704a0544ae79cb022288d
Author: Catherine Mullings <catmullings@chromium.org>
Date: Mon Oct 02 23:18:19 2017

Extensions: Refactor use of range check of custom extension menu ids

Refactor checks of whether a menu item id is an extension custom id or
not. The static ContextMenuMatcher::IsExtensionsCustomCommandId()
function is used instead.

Bug:  765829 
Change-Id: I2143e9449beb82a2e6580d82e86055b83c6ca41c
Reviewed-on: https://chromium-review.googlesource.com/695718
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: catmullings <catmullings@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505844}
[modify] https://crrev.com/8e7c1213f30e0f24539704a0544ae79cb022288d/chrome/browser/extensions/api/context_menus/context_menu_apitest.cc
[modify] https://crrev.com/8e7c1213f30e0f24539704a0544ae79cb022288d/chrome/browser/extensions/context_menu_matcher.cc
[modify] https://crrev.com/8e7c1213f30e0f24539704a0544ae79cb022288d/chrome/browser/extensions/extension_context_menu_model.cc
[modify] https://crrev.com/8e7c1213f30e0f24539704a0544ae79cb022288d/chrome/browser/extensions/extension_context_menu_model_unittest.cc

Sign in to add a comment