Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1106193003:
Allow user to manually fetch remote commands (Closed)
DescriptionAllow user to manually fetch remote commands
Currently remote commands fetch depends solely on invalidation, evaluate and
possibly remove this after invalidation service is landed and tested.
BUG=480982
Committed: https://crrev.com/610f47342c8d725e239513ac17727a176d161495
Cr-Commit-Position: refs/heads/master@{#327765}
Committed: https://crrev.com/483c224aaa3cbb52d1db3b6ba00b902d0c716474
Cr-Commit-Position: refs/heads/master@{#330082}
Patch Set 1 #
Total comments: 6
Patch Set 2 : fixes addressing #6 #
Total comments: 2
Patch Set 3 : fix addressing #8 #Patch Set 4 : rebase; fixes #
Messages
Total messages: 24 (8 generated)
binjin@chromium.org changed reviewers: + bartfab@chromium.org
Bartosz, Please take a look at this CL
binjin@chromium.org changed reviewers: + estade@chromium.org
estade: Could you do an owner review first? I will land after bartfab@ LGTMed this.
On 2015/04/28 17:42:23, binjin wrote: > estade: Could you do an owner review first? I will land after bartfab@ LGTMed > this. I'd rather give an owners review after bartfab's review, in case something changes during his review.
lgtm https://codereview.chromium.org/1106193003/diff/1/chrome/browser/ui/webui/pol... File chrome/browser/ui/webui/policy_ui.cc (right): https://codereview.chromium.org/1106193003/diff/1/chrome/browser/ui/webui/pol... chrome/browser/ui/webui/policy_ui.cc:814: // TODO(binjin): evaluate and possibly remove this after invalidation Nit: Please reference a bug number. https://codereview.chromium.org/1106193003/diff/1/chrome/browser/ui/webui/pol... chrome/browser/ui/webui/policy_ui.cc:816: policy::BrowserPolicyConnectorChromeOS* connector = Nit: #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h" https://codereview.chromium.org/1106193003/diff/1/chrome/browser/ui/webui/pol... chrome/browser/ui/webui/policy_ui.cc:817: g_browser_process->platform_part()->browser_policy_connector_chromeos(); Nit: #include "chrome/browser/browser_process_platform_part.h"
Evan, please take a look, thanks https://codereview.chromium.org/1106193003/diff/1/chrome/browser/ui/webui/pol... File chrome/browser/ui/webui/policy_ui.cc (right): https://codereview.chromium.org/1106193003/diff/1/chrome/browser/ui/webui/pol... chrome/browser/ui/webui/policy_ui.cc:814: // TODO(binjin): evaluate and possibly remove this after invalidation On 2015/04/29 15:21:38, bartfab wrote: > Nit: Please reference a bug number. Done. https://codereview.chromium.org/1106193003/diff/1/chrome/browser/ui/webui/pol... chrome/browser/ui/webui/policy_ui.cc:816: policy::BrowserPolicyConnectorChromeOS* connector = On 2015/04/29 15:21:38, bartfab wrote: > Nit: #include > "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h" It's already included, inside defined(OS_CHROMEOS) guard. https://codereview.chromium.org/1106193003/diff/1/chrome/browser/ui/webui/pol... chrome/browser/ui/webui/policy_ui.cc:817: g_browser_process->platform_part()->browser_policy_connector_chromeos(); On 2015/04/29 15:21:38, bartfab wrote: > Nit: #include "chrome/browser/browser_process_platform_part.h" Done.
Still LGTM. https://codereview.chromium.org/1106193003/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/policy_ui.cc (right): https://codereview.chromium.org/1106193003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/policy_ui.cc:19: #include "chrome/browser/browser_process_platform_part.h" Nit: You only need this #if defined(OS_CHROMEOS)
https://codereview.chromium.org/1106193003/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/policy_ui.cc (right): https://codereview.chromium.org/1106193003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/policy_ui.cc:19: #include "chrome/browser/browser_process_platform_part.h" On 2015/04/29 17:27:54, bartfab wrote: > Nit: You only need this #if defined(OS_CHROMEOS) Done.
lgtm
The CQ bit was checked by binjin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bartfab@chromium.org Link to the patchset: https://codereview.chromium.org/1106193003/#ps40001 (title: "fix addressing #8")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1106193003/40001
The CQ bit was unchecked by binjin@chromium.org
The CQ bit was checked by binjin@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1106193003/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/610f47342c8d725e239513ac17727a176d161495 Cr-Commit-Position: refs/heads/master@{#327765}
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/1132223003/ by binjin@chromium.org. The reason for reverting is: crash occasionally on CrOS.
The CQ bit was checked by binjin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bartfab@chromium.org, estade@chromium.org Link to the patchset: https://codereview.chromium.org/1106193003/#ps60001 (title: "rebase; fixes")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1106193003/60001
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/483c224aaa3cbb52d1db3b6ba00b902d0c716474 Cr-Commit-Position: refs/heads/master@{#330082} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
