Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2253613003:
Fix MediaRouterContextualMenuUnitTest (Closed)
DescriptionFix MediaRouterContextualMenuUnitTest
This CL resolves the compliation error in the unit test introduced
by https://codereview.chromium.org/2155293002.
TBR=msw@chromium.org
BUG=638300
Committed: https://crrev.com/0d3d4235ebcd37fa56b05517c9d97c018e8c0f5f
Cr-Commit-Position: refs/heads/master@{#412332}
Patch Set 1 #
Messages
Total messages: 27 (16 generated)
The CQ bit was checked by takumif@chromium.org to run a CQ dry run
The CQ bit was unchecked by takumif@chromium.org
Description was changed from ========== Fix MediaRouterContextualMenuUnitTest This CL resolves the compliation error in the unit test introduced by https://codereview.chromium.org/2155293002. BUG=638300 ========== to ========== Fix MediaRouterContextualMenuUnitTest This CL resolves the compliation error in the unit test introduced by https://codereview.chromium.org/2155293002. TBR=msw@chromium.org BUG=638300 ==========
takumif@chromium.org changed reviewers: + msw@chromium.org
The CQ bit was checked by takumif@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
lgtm
The CQ bit was checked by takumif@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by takumif@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was unchecked by takumif@chromium.org
The CQ bit was checked by takumif@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Fix MediaRouterContextualMenuUnitTest This CL resolves the compliation error in the unit test introduced by https://codereview.chromium.org/2155293002. TBR=msw@chromium.org BUG=638300 ========== to ========== Fix MediaRouterContextualMenuUnitTest This CL resolves the compliation error in the unit test introduced by https://codereview.chromium.org/2155293002. TBR=msw@chromium.org BUG=638300 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Fix MediaRouterContextualMenuUnitTest This CL resolves the compliation error in the unit test introduced by https://codereview.chromium.org/2155293002. TBR=msw@chromium.org BUG=638300 ========== to ========== Fix MediaRouterContextualMenuUnitTest This CL resolves the compliation error in the unit test introduced by https://codereview.chromium.org/2155293002. TBR=msw@chromium.org BUG=638300 Committed: https://crrev.com/0d3d4235ebcd37fa56b05517c9d97c018e8c0f5f Cr-Commit-Position: refs/heads/master@{#412332} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/0d3d4235ebcd37fa56b05517c9d97c018e8c0f5f Cr-Commit-Position: refs/heads/master@{#412332}
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/2260873003/ by takumif@chromium.org. The reason for reverting is: Reverting this CL as well as the other CL[1] this was meant to fix. They are causing crashes and need a redesign. [1] https://codereview.chromium.org/2155293002.
Message was sent while issue was closed.
Hey Mike, I just wanted to thank you for approving this patch so quickly the other day. Unfortunately there were other problems and I need to revert it :( On Fri, Aug 19, 2016 at 3:11 PM, <takumif@chromium.org> wrote: > Reviewers: msw > CL: https://codereview.chromium.org/2253613003/ > > Message: > A revert of this CL (patchset #1 id:1) has been created in > https://codereview.chromium.org/2260873003/ by takumif@chromium.org. > > The reason for reverting is: Reverting this CL as well as the other CL[1] > this > was meant to fix. They are causing crashes and need a redesign. > > [1] https://codereview.chromium.org/2155293002. > > Description: > Fix MediaRouterContextualMenuUnitTest > > This CL resolves the compliation error in the unit test introduced > by https://codereview.chromium.org/2155293002. > > TBR=msw@chromium.org > BUG=638300 > > Committed: https://crrev.com/0d3d4235ebcd37fa56b05517c9d97c018e8c0f5f > Cr-Commit-Position: refs/heads/master@{#412332} > > Base URL: https://chromium.googlesource.com/chromium/src.git@master > > Affected files (+2, -1 lines): > M chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc > > > Index: chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc > diff --git a/chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc > b/chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc > index add326148d8fdc0946b0cd0b042734b593e8c412.. > e628cd72c51b8b7d12654d448798ce545996a11d 100644 > --- a/chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc > +++ b/chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc > @@ -35,6 +35,7 @@ class MediaRouterContextualMenuUnitTest : public > BrowserWithTestWindowTest { > > SigninManagerBase* signin_manager() { return signin_manager_; } > ui::SimpleMenuModel* model() { return model_; } > + MediaRouterAction* action() { return action_.get(); } > > private: > std::unique_ptr<BrowserActionTestUtil> browser_action_test_util_; > @@ -96,7 +97,7 @@ TEST_F(MediaRouterContextualMenuUnitTest, Basic) { > TEST_F(MediaRouterContextualMenuUnitTest, ToggleCloudServicesItem) { > // The Media Router Action has a getter for the model, but not the > delegate. > // Create the MediaRouterContextualMenu ui::SimpleMenuModel::Delegate here. > - MediaRouterContextualMenu menu(browser()); > + MediaRouterContextualMenu menu(browser(), action()); > > // Set up an authenticated account such that the cloud services menu item > is > // surfaced. Whether or not it is surfaced is tested in the "Basic" test. > > > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Message was sent while issue was closed.
On 2016/08/19 22:14:51, chromium-reviews wrote: > Hey Mike, I just wanted to thank you for approving this patch so quickly > the other day. Unfortunately there were other problems and I need to revert > it :( No worries, when you're ready, upload this same exact patch to a new codereview issue, then upload a separate updated patch set that changes anything as needed. That is the standard practice for relanding CLs, to make diffing easier for reviewers, but it should be pretty easy in the case of this particular CL regardless. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
