Refactorise the policy_path_parser framework

Refactorise the policy_path_parser framework

This CL performs a refactorisation of the policy path parser framework.

BUG=352627
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=261370

[kaliamoorthi, 2014-03-31 11:32:50]

I reduced the static initialization that happens in the new code.

PTAL

[pastarmovj, 2014-04-01 09:17:28]

this looks pretty good. Add the comments we discussed and consider the
suggestion I have in the code and we can give it a try.

https://codereview.chromium.org/214233003/diff/20001/chrome/browser/policy/po...
File chrome/browser/policy/policy_path_parser_win.cc (right):

https://codereview.chromium.org/214233003/diff/20001/chrome/browser/policy/po...
chrome/browser/policy/policy_path_parser_win.cc:47: #define
WRAP_GET_FORLDER_FUNCTION(FunctionName, WindowsId) \
I think FolderId is more appropriate name here.

[kaliamoorthi, 2014-04-01 13:43:17]

PTAL

https://codereview.chromium.org/214233003/diff/20001/chrome/browser/policy/po...
File chrome/browser/policy/policy_path_parser_win.cc (right):

https://codereview.chromium.org/214233003/diff/20001/chrome/browser/policy/po...
chrome/browser/policy/policy_path_parser_win.cc:47: #define
WRAP_GET_FORLDER_FUNCTION(FunctionName, WindowsId) \
On 2014/04/01 09:17:29, pastarmovj wrote:
> I think FolderId is more appropriate name here.

Done.

[pastarmovj, 2014-04-01 13:51:02]

lgtm

[kaliamoorthi, 2014-04-01 13:51:14]

The CQ bit was checked by kaliamoorthi@chromium.org

[commit-bot: I haz the power, 2014-04-01 13:51:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaliamoorthi@chromium.org/214233003/40001

[commit-bot: I haz the power, 2014-04-01 22:18:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaliamoorthi@chromium.org/214233003/40001

[commit-bot: I haz the power, 2014-04-01 22:20:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-01 22:20:57]

Internal error: failed to checkout. Please try again.

[kaliamoorthi, 2014-04-02 09:14:15]

The CQ bit was checked by kaliamoorthi@chromium.org

[commit-bot: I haz the power, 2014-04-02 09:14:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaliamoorthi@chromium.org/214233003/40001

[commit-bot: I haz the power, 2014-04-02 10:00:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-02 10:00:11]

Retried try job too often on win_rel for step(s) net_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[kaliamoorthi, 2014-04-02 10:50:01]

The CQ bit was checked by kaliamoorthi@chromium.org

[commit-bot: I haz the power, 2014-04-02 10:50:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaliamoorthi@chromium.org/214233003/40001

[commit-bot: I haz the power, 2014-04-02 17:13:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-02 17:13:18]

Retried try job too often on android_aosp for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_ao...

[kaliamoorthi, 2014-04-03 08:23:28]

The CQ bit was checked by kaliamoorthi@chromium.org

[commit-bot: I haz the power, 2014-04-03 08:24:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaliamoorthi@chromium.org/214233003/40001

[commit-bot: I haz the power, 2014-04-03 11:33:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-03 11:33:41]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[kaliamoorthi, 2014-04-03 12:18:03]

The CQ bit was checked by kaliamoorthi@chromium.org

[commit-bot: I haz the power, 2014-04-03 12:18:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaliamoorthi@chromium.org/214233003/40001

[commit-bot: I haz the power, 2014-04-03 12:21:08]

Change committed as 261370

[pfeldman, 2014-04-03 14:35:38]

A revert of this CL has been created in
https://codereview.chromium.org/223993002/ by pfeldman@chromium.org.

The reason for reverting is: Speculative revert due to perf regression:
http://build.chromium.org/p/chromium/buildstatus?builder=Linux&number=48755.

[bungeman-skia, 2014-04-03 14:35:41]

On 2014/04/03 12:21:08, I haz the power (commit-bot) wrote:
> Change committed as 261370

This appears to have added a static initializer (policy_path_parser_linux.cc
__i686.get_pc_thunk.cx), causing sizes on linux to fail. See
http://build.chromium.org/p/chromium/buildstatus?builder=Linux&number=48755 .
Investigating.

[bungeman-skia, 2014-04-03 15:00:10]

A revert of this CL has been created in
https://codereview.chromium.org/224023002/ by bungeman@google.com.

The reason for reverting is: Added a static initializer
(policy_path_parser_linux.cc__i686.get_pc_thunk.cx), causing sizes on linux to
fail. See
http://build.chromium.org/p/chromium/buildstatus?builder=Linux&number=48755 ..

[bungeman-skia, 2014-04-03 15:10:22]

Looks like pfeldman and I both set off to revert around the same time, and his
made it in first. Just wanted to comment on the reason.

https://codereview.chromium.org/214233003/diff/40001/chrome/browser/policy/po...
File chrome/browser/policy/policy_path_parser.cc (right):

https://codereview.chromium.org/214233003/diff/40001/chrome/browser/policy/po...
chrome/browser/policy/policy_path_parser.cc:58: val_func_ptr =
internal::kVariableNameAndValueCallbacks[i].value_func_ptr;
These two lines appear to cause the thunk to be generated. In order to to access
these globals in PIC, the linker inserts a base pointer initializer.