Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1490503002:
MD Settings: Adding unit test for ResetSettingsHandler. (Closed)
|
Created:
5 years ago by dpapad Modified:
5 years ago CC:
chromium-reviews, dbeam+watch-settings_chromium.org, michaelpg+watch-md-settings_chromium.org, stevenjb+watch-md-settings_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMD Settings: Adding unit test for ResetSettingsHandler.
BUG=546840
Committed: https://crrev.com/86b329a6a0dd95fdecf769e1eb868b0ce79243f6
Cr-Commit-Position: refs/heads/master@{#363029}
Patch Set 1 #Patch Set 2 : Remove gmock usage #Patch Set 3 : Cleanup #
Total comments: 25
Patch Set 4 : Addressing comments. #
Total comments: 20
Patch Set 5 : Address comments #
Total comments: 7
Patch Set 6 : Addressing comments #
Messages
Total messages: 27 (9 generated)
Patchset #3 (id:40001) has been deleted
Patchset #3 (id:60001) has been deleted
Patchset #3 (id:80001) has been deleted
dpapad@chromium.org changed reviewers: + battre@chromium.org, dbeam@chromium.org, engedy@chromium.org
@battre, engedy: Please review profile_resetter.h @dbeam: Please remaining changes.
https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler.cc (right): https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler.cc:58: #if defined(OS_CHROMEOS) so.... have you tried this on chromeos? :) https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler.cc:68: // class much easier. // Inject |allow_powerwash| for testing. or just remove https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler.h (right): https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler.h:39: explicit ResetSettingsHandler(Profile* profile, bool allow_powerwash); drop explicit, make private or protected https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler.h:49: virtual ProfileResetter* GetResetter(); protected https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc (right): https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:20: /* Mock version of ProfileResetter to be used in tests. */ // Single line comment. https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:43: int resets_ = 0; nit: size_t https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:68: scoped_ptr<TestingProfile> profile_; why should the handler own the profile? this should be owned by the fixture https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:88: scoped_ptr<TestingResetSettingsHandler> handler_; nit: instead of protected member, private member + public accessor to mutable pointer https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:88: scoped_ptr<TestingResetSettingsHandler> handler_; why does this need to be a scoped_ptr? just initialize these in the right order: public: ResetSettingsHandlerTest : ui_thread_(content::BrowserThread::UI, &message_loop_), handler_(&profile_, false, &web_ui_) {} private: base::MessageLoopForUI message_loop_; content::TestBrowserThread ui_thread_; TestingProfile profile_; content::TestWebUI web_ui_; TestResetSettingsHandler handler_; https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:89: scoped_ptr<content::TestWebUI> web_ui_; why does this need to be a scoped_ptr? https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:104: web_ui_->call_data()[0]->function_name()); doesn't this effectively check that resets_ >= 1?
https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/profile... File chrome/browser/profile_resetter/profile_resetter.h (right): https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/profile... chrome/browser/profile_resetter/profile_resetter.h:68: virtual bool IsActive() const; Why do you make these virtual? I don't see them mocked or overridden anywhere.
chrome/browser/profile_resetter/profile_resetter.h LGTM. With regard to *not* this CL, but the entire implementation of the new handler, at one point you should probably run it by vasilii@, who originally wrote most of it. He should be able to spot anything that might look even the tiniest bit out of place. https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/profile... File chrome/browser/profile_resetter/profile_resetter.h (right): https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/profile... chrome/browser/profile_resetter/profile_resetter.h:68: virtual bool IsActive() const; On 2015/12/01 09:55:19, battre wrote: > Why do you make these virtual? I don't see them mocked or overridden anywhere. They seem to be overridden by the mock in the unittest. Adding tests for WebUI handlers is something I've been looking forward to for a long time, so I personally very much welcome this change.
https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc (right): https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:27: bool IsActive() const override { battre@: here ^ https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:33: const base::Closure& callback) override { battre@: and here ^
https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler.cc (right): https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler.cc:58: #if defined(OS_CHROMEOS) On 2015/12/01 at 04:27:37, Dan Beam wrote: > so.... have you tried this on chromeos? :) Fixed. Thanks for catching. https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler.cc:68: // class much easier. On 2015/12/01 at 04:27:37, Dan Beam wrote: > // Inject |allow_powerwash| for testing. > > or just remove Done. https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler.h (right): https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler.h:39: explicit ResetSettingsHandler(Profile* profile, bool allow_powerwash); On 2015/12/01 at 04:27:37, Dan Beam wrote: > drop explicit, make private or protected Done. https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler.h:49: virtual ProfileResetter* GetResetter(); On 2015/12/01 at 04:27:37, Dan Beam wrote: > protected Done. https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc (right): https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:20: /* Mock version of ProfileResetter to be used in tests. */ On 2015/12/01 at 04:27:37, Dan Beam wrote: > // Single line comment. Done. https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:43: int resets_ = 0; On 2015/12/01 at 04:27:37, Dan Beam wrote: > nit: size_t Done. https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:88: scoped_ptr<TestingResetSettingsHandler> handler_; On 2015/12/01 at 04:27:37, Dan Beam wrote: > why does this need to be a scoped_ptr? just initialize these in the right order: > > public: > ResetSettingsHandlerTest > : ui_thread_(content::BrowserThread::UI, &message_loop_), > handler_(&profile_, false, &web_ui_) {} > > private: > base::MessageLoopForUI message_loop_; > content::TestBrowserThread ui_thread_; > TestingProfile profile_; > content::TestWebUI web_ui_; > TestResetSettingsHandler handler_; Done. Also removed usage of deprecated TestBrowserThread (see https://code.google.com/p/chromium/codesearch#chromium/src/content/public/tes...) and replaced with content::TestBrowserThreadBundle. https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:89: scoped_ptr<content::TestWebUI> web_ui_; On 2015/12/01 at 04:27:37, Dan Beam wrote: > why does this need to be a scoped_ptr? Removed. https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:104: web_ui_->call_data()[0]->function_name()); On 2015/12/01 at 04:27:37, Dan Beam wrote: > doesn't this effectively check that resets_ >= 1? It does. The assertion at line 101 checks that resets_ == 1, not just >=1.
https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc (right): https://codereview.chromium.org/1490503002/diff/100001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:104: web_ui_->call_data()[0]->function_name()); On 2015/12/01 19:06:04, dpapad wrote: > On 2015/12/01 at 04:27:37, Dan Beam wrote: > > doesn't this effectively check that resets_ >= 1? > > It does. The assertion at line 101 checks that resets_ == 1, not just >=1. ok https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler.h (right): https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler.h:64: void OnResetProfileSettingsDone(bool send_feedback); nit: revert moving of this method to maintain the blame https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler.h:88: Profile* profile_; nit: Profile* const profile_; https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc (right): https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:36: int resets() const { size_t https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:43: remove extra \n https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:55: set_web_ui(nullptr); why is this necessary? destruction order issue? if so: you could just do handler_.reset() instead, which is better IMO https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:58: MockProfileResetter* GetResetter() override { how is this override working if you're using MockProfileResetter* instead of ProfileResetter*? https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:91: remove extra \n https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:98: EXPECT_EQ(1, handler()->GetResetter()->resets()); nit: add public: size_t resets() const { return resetter_.resets(); } to TestingResetSettingsHandler and check EXPECT_EQ(1, handler()->resets()); instead (reduces unneeded access) https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:103: remove extra \n
https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler.h (right): https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler.h:64: void OnResetProfileSettingsDone(bool send_feedback); On 2015/12/02 at 04:40:07, Dan Beam wrote: > nit: revert moving of this method to maintain the blame Done. https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler.h:88: Profile* profile_; On 2015/12/02 at 04:40:08, Dan Beam wrote: > nit: Profile* const profile_; Done. https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc (right): https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:36: int resets() const { On 2015/12/02 at 04:40:08, Dan Beam wrote: > size_t Done. https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:43: On 2015/12/02 at 04:40:08, Dan Beam wrote: > remove extra \n Styleguide does not prohibit double blank lines for separating functions/classes (https://google.github.io/styleguide/cppguide.html#Vertical_Whitespace). I find that useful since it helps visually parse a wall of text, by easily identifying start/end of functions and classes (given that double blank lines do not appear within a function). https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:55: set_web_ui(nullptr); On 2015/12/02 at 04:40:08, Dan Beam wrote: > why is this necessary? destruction order issue? if so: you could just do handler_.reset() instead, which is better IMO Removed, it is not necessary, removed. It was there because I copied it from https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/.... https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:58: MockProfileResetter* GetResetter() override { On 2015/12/02 at 04:40:08, Dan Beam wrote: > how is this override working if you're using MockProfileResetter* instead of ProfileResetter*? Because C++ allows covariant return types, see example at http://www.lwithers.me.uk/articles/covariant.html. https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:91: On 2015/12/02 at 04:40:08, Dan Beam wrote: > remove extra \n Done. https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:98: EXPECT_EQ(1, handler()->GetResetter()->resets()); On 2015/12/02 at 04:40:08, Dan Beam wrote: > nit: add > > public: > size_t resets() const { return resetter_.resets(); } > > to TestingResetSettingsHandler and check > > EXPECT_EQ(1, handler()->resets()); > > instead (reduces unneeded access) Done. https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:103: On 2015/12/02 at 04:40:08, Dan Beam wrote: > remove extra \n Done.
https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc (right): https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:43: On 2015/12/02 19:55:22, dpapad wrote: > On 2015/12/02 at 04:40:08, Dan Beam wrote: > > remove extra \n > > Styleguide does not prohibit double blank lines for separating functions/classes > (https://google.github.io/styleguide/cppguide.html#Vertical_Whitespace). I find > that useful since it helps visually parse a wall of text, by easily identifying > start/end of functions and classes (given that double blank lines do not appear > within a function). the style guide hurts your case by strongly encouraging you NOT to add extra vertical whitespace. this is not closure JS, this is Chrome's C++ https://codereview.chromium.org/1490503002/diff/140001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc (right): https://codereview.chromium.org/1490503002/diff/140001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:59: MockProfileResetter* GetResetter() override { ProfileResetter or find precedence for doing this in Chrome https://codereview.chromium.org/1490503002/diff/140001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:97: EXPECT_EQ((size_t)1, handler()->resets()); 1u
https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc (right): https://codereview.chromium.org/1490503002/diff/120001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:43: On 2015/12/02 at 20:08:36, Dan Beam wrote: > On 2015/12/02 19:55:22, dpapad wrote: > > On 2015/12/02 at 04:40:08, Dan Beam wrote: > > > remove extra \n > > > > Styleguide does not prohibit double blank lines for separating functions/classes > > (https://google.github.io/styleguide/cppguide.html#Vertical_Whitespace). I find > > that useful since it helps visually parse a wall of text, by easily identifying > > start/end of functions and classes (given that double blank lines do not appear > > within a function). > > the style guide hurts your case by strongly encouraging you NOT to add extra vertical whitespace. this is not closure JS, this is Chrome's C++ Removed per request, I don't mind that much either way. For discussion's sake, I have found the double blank line useful (regardless of language, since visual parsing of start/end of functions/classes applies to both JS and C++). This really boils down to what is considered "extra" whitespace. Specifically the sentence "Of course, readability can suffer from code being too dense as well as too spread out, so use your judgement. But in general, minimize use of vertical whitespace". I interpret that as "use whitespace if you think it improves readability". The styleguide wording also seems fairly mild, so I don't interpret it as "strongly encourages NOT to add extra whitespace". https://codereview.chromium.org/1490503002/diff/140001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc (right): https://codereview.chromium.org/1490503002/diff/140001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:59: MockProfileResetter* GetResetter() override { On 2015/12/02 at 20:08:36, Dan Beam wrote: > ProfileResetter or find precedence for doing this in Chrome Done. Changed to ProfileResetter. MockProfileResetter* was needed when I was calling handler()->GetResetter()->resets(); from the test. Now that we wrapped it with handler()->resets(); it does not need to retur MockProfileResetter*. https://codereview.chromium.org/1490503002/diff/140001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:97: EXPECT_EQ((size_t)1, handler()->resets()); On 2015/12/02 at 20:08:36, Dan Beam wrote: > 1u Done.
lgtm https://codereview.chromium.org/1490503002/diff/140001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc (right): https://codereview.chromium.org/1490503002/diff/140001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:59: MockProfileResetter* GetResetter() override { On 2015/12/02 21:33:35, dpapad wrote: > On 2015/12/02 at 20:08:36, Dan Beam wrote: > > ProfileResetter or find precedence for doing this in Chrome > > Done. Changed to ProfileResetter. MockProfileResetter* was needed when I was > calling > > handler()->GetResetter()->resets(); > from the test. Now that we wrapped it with > handler()->resets(); > it does not need to retur MockProfileResetter*. i know :), which is part of why I suggested that
https://codereview.chromium.org/1490503002/diff/140001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc (right): https://codereview.chromium.org/1490503002/diff/140001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:70: // The fixture for testing class Foo. lol whoops
https://codereview.chromium.org/1490503002/diff/140001/chrome/browser/ui/webu... File chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc (right): https://codereview.chromium.org/1490503002/diff/140001/chrome/browser/ui/webu... chrome/browser/ui/webui/settings/reset_settings_handler_unittest.cc:70: // The fixture for testing class Foo. On 2015/12/02 at 21:36:52, Dan Beam wrote: > lol whoops Already removed in latest patch (#6).
The CQ bit was checked by dpapad@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from engedy@chromium.org Link to the patchset: https://codereview.chromium.org/1490503002/#ps160001 (title: "Addressing comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1490503002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1490503002/160001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by dpapad@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1490503002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1490503002/160001
Message was sent while issue was closed.
Committed patchset #6 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== MD Settings: Adding unit test for ResetSettingsHandler. BUG=546840 ========== to ========== MD Settings: Adding unit test for ResetSettingsHandler. BUG=546840 Committed: https://crrev.com/86b329a6a0dd95fdecf769e1eb868b0ce79243f6 Cr-Commit-Position: refs/heads/master@{#363029} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/86b329a6a0dd95fdecf769e1eb868b0ce79243f6 Cr-Commit-Position: refs/heads/master@{#363029} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
