Sending over to groby@ as patch is in the works.

Not sure if I'm the best choice, but sure :)

shrike@: Do we have *any* tests that handle menu commands?  render_view_browsertest_mac.mm sort of does that, but seems to pass the desired edit commands down to the RenderViewImpl ahead of time - which would prevent us testing if the right message is sent.

Hello groby@,

I'm not sure if this is what you're looking for, but in menu_controller_unittest.mm, TEST_F(MenuControllerTest, Execute) simulates a menu command invocation using performSelector:.

Alas, not quite - this is a unittest that menus work. I'm looking for a browsertest so I can test that pressing Cmd-Shift-; actually opens the spelling panel. (Bonus point, I need to test it both with focus in the Omnibox, and focus in the renderer, because they are separate paths)

Getting focus to the omnibox is straightforward, I think. I'm not sure how to do that with web content - perhaps the Accessibility APIs to simulate a11y tab presses to a textfield in web content?

For pressing the menu item I keep thinking of solutions that are further downstream than you want. I think you might be able to simulate a cmd key press by sending -postEvent:atStart: to NSApp (and then running the run loop).

Hello groby@,

Are you working on this? If not, can you start, or perhaps assign it back to dtapuska@? The failure this test guards against was pretty bad, and I don't want to ship to stable without this test existing. It's sort of too late for M63 but I'm marking this RBS M64.

I'm currently not able to work on the test, so throwing back to dtapuska

Fix is in https://chromium-review.googlesource.com/c/chromium/src/+/759658 - waiting for dtapuska review as well. If you want both combined into one patch, feel free to just lift that code from my CL. 

dtapuska@,Could you please take a look into this issue as it is marked as stable blocker.

Thanks..!

Friendly ping to get an update on this issue as it is marked as stable blocker.

Thanks..

I've been waiting on a review from groby@ since last Friday on this. I believe she was OOO hopefully today it will be reviewed.

dtapuska@,
Could you please let us know the update on this issue?

Thanks..!

What is the status of this test? Without a working test, how can we know that this feature is not broken right now?

The change adding the test has existed for review for about a week and a half. Unfortunately the OWNERS (groby@) were OOO and the other OWNER didn't feel comfortable reviewing it. It is now approved however I am OOO so unable to land.

Since it has branched and the issue existed in a stable branch already I don't know how adding a test meets any RSB or merge criteria. This spellcheck code was written long ago and I'm surprised it didn't have sufficient tests from the beginning.

Can you please provide a reference to the criteria you are using here? Why fixing this wasn't a RBS issue against 61 or 62 (the branches it was found in) yet is a RBS against 63 for a test?

> It is now approved however I am OOO so unable to land

When can you land it?

> Can you please provide a reference to the criteria you are using here? Why fixing this wasn't a RBS issue against 61 or 62 (the branches it was found in) yet is a RBS against 63 for a test?

I believe this regressed in 61 or 62 but was only found recently. In other words, I don't believe we shipped M61 knowing this bug existed. And it's a pretty bad bug we don't want to be flying blind about in the future.

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

commit 093acc6f3b45b12589dd6c116a2ef949b1ebd285
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Wed Dec 06 15:17:52 2017

Add unittest for showing spelling panel on mac via EditCommand

Repurpose spell checking code that was testing for the mojo channel
to common code. Write a test that causes the showGuessPanel selector
to be sent and verify the panel is requested to be shown.

BUG= 782836 

Change-Id: I9d8ac59ac3bf96bbcd7e538f794eac71e0bc7553
Reviewed-on: https://chromium-review.googlesource.com/786260
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Rachel Blum <groby@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522088}
[modify] https://crrev.com/093acc6f3b45b12589dd6c116a2ef949b1ebd285/chrome/browser/BUILD.gn
[modify] https://crrev.com/093acc6f3b45b12589dd6c116a2ef949b1ebd285/chrome/browser/chrome_site_per_process_browsertest.cc
[add] https://crrev.com/093acc6f3b45b12589dd6c116a2ef949b1ebd285/chrome/browser/spellchecker/spellcheck_mac_view_browsertest.mm
[add] https://crrev.com/093acc6f3b45b12589dd6c116a2ef949b1ebd285/chrome/browser/spellchecker/test/spellcheck_content_browser_client.cc
[add] https://crrev.com/093acc6f3b45b12589dd6c116a2ef949b1ebd285/chrome/browser/spellchecker/test/spellcheck_content_browser_client.h
[add] https://crrev.com/093acc6f3b45b12589dd6c116a2ef949b1ebd285/chrome/browser/spellchecker/test/spellcheck_mock_panel_host.cc
[add] https://crrev.com/093acc6f3b45b12589dd6c116a2ef949b1ebd285/chrome/browser/spellchecker/test/spellcheck_mock_panel_host.h
[modify] https://crrev.com/093acc6f3b45b12589dd6c116a2ef949b1ebd285/chrome/test/BUILD.gn

Your change meets the bar and is auto-approved for M64. Please go ahead and merge the CL to branch 3282 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

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

commit d85990abe19baf985ce95c7cd882ede90ba27902
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Fri Dec 08 14:47:30 2017

Add unittest for showing spelling panel on mac via EditCommand

Repurpose spell checking code that was testing for the mojo channel
to common code. Write a test that causes the showGuessPanel selector
to be sent and verify the panel is requested to be shown.

BUG= 782836 
TBR=dtapuska@chromium.org

(cherry picked from commit 093acc6f3b45b12589dd6c116a2ef949b1ebd285)

Change-Id: I9d8ac59ac3bf96bbcd7e538f794eac71e0bc7553
Reviewed-on: https://chromium-review.googlesource.com/786260
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Rachel Blum <groby@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#522088}
Reviewed-on: https://chromium-review.googlesource.com/817335
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#95}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/d85990abe19baf985ce95c7cd882ede90ba27902/chrome/browser/BUILD.gn
[modify] https://crrev.com/d85990abe19baf985ce95c7cd882ede90ba27902/chrome/browser/chrome_site_per_process_browsertest.cc
[add] https://crrev.com/d85990abe19baf985ce95c7cd882ede90ba27902/chrome/browser/spellchecker/spellcheck_mac_view_browsertest.mm
[add] https://crrev.com/d85990abe19baf985ce95c7cd882ede90ba27902/chrome/browser/spellchecker/test/spellcheck_content_browser_client.cc
[add] https://crrev.com/d85990abe19baf985ce95c7cd882ede90ba27902/chrome/browser/spellchecker/test/spellcheck_content_browser_client.h
[add] https://crrev.com/d85990abe19baf985ce95c7cd882ede90ba27902/chrome/browser/spellchecker/test/spellcheck_mock_panel_host.cc
[add] https://crrev.com/d85990abe19baf985ce95c7cd882ede90ba27902/chrome/browser/spellchecker/test/spellcheck_mock_panel_host.h
[modify] https://crrev.com/d85990abe19baf985ce95c7cd882ede90ba27902/chrome/test/BUILD.gn