Mark methods with override in pdf/

Mark methods with override in pdf/

Fix some small nits along the way.

Committed: https://crrev.com/98638620100303034cd94c7a02e8c64be0bd93b7
Cr-Commit-Position: refs/heads/master@{#367109}

[Lei Zhang, 2015-12-29 19:02:15]

thestig@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2015-12-29 19:03:28]

https://codereview.chromium.org/1554533002/diff/20001/pdf/out_of_process_inst...
File pdf/out_of_process_instance.h (right):

https://codereview.chromium.org/1554533002/diff/20001/pdf/out_of_process_inst...
pdf/out_of_process_instance.h:77: pp::Var GetLinkAtPosition(const pp::Point&
point);
...shouldn't these be marked override?

[Nico, 2015-12-29 19:05:40]

https://codereview.chromium.org/1554533002/diff/20001/tools/clang/plugins/Chr...
File tools/clang/plugins/ChromeClassTester.cpp (left):

https://codereview.chromium.org/1554533002/diff/20001/tools/clang/plugins/Chr...
tools/clang/plugins/ChromeClassTester.cpp:234:
banned_directories_.emplace("/pdf/");
In general, we put changes like this behind a flag. The plugin is bundled with
clang, so this will have an effect the next time we roll clang. Rolling clang is
already very hard, so we don't want to be blocked on a certain directory having
to be plugin-clean for the roll. So put this behind a flag, wait for the roll,
then toggle the flag async after the roll. (And then the flag can be removed
again.)

[Lei Zhang, 2015-12-29 19:14:51]

https://codereview.chromium.org/1554533002/diff/20001/pdf/out_of_process_inst...
File pdf/out_of_process_instance.h (right):

https://codereview.chromium.org/1554533002/diff/20001/pdf/out_of_process_inst...
pdf/out_of_process_instance.h:77: pp::Var GetLinkAtPosition(const pp::Point&
point);
On 2015/12/29 19:03:27, Nico wrote:
> ...shouldn't these be marked override?

No, they implement the PPAPI functions from ppapi/c/private/ppp_pdf.h.

https://codereview.chromium.org/1554533002/diff/20001/tools/clang/plugins/Chr...
File tools/clang/plugins/ChromeClassTester.cpp (left):

https://codereview.chromium.org/1554533002/diff/20001/tools/clang/plugins/Chr...
tools/clang/plugins/ChromeClassTester.cpp:234:
banned_directories_.emplace("/pdf/");
On 2015/12/29 19:05:40, Nico wrote:
> In general, we put changes like this behind a flag. The plugin is bundled with
> clang, so this will have an effect the next time we roll clang. Rolling clang
is
> already very hard, so we don't want to be blocked on a certain directory
having
> to be plugin-clean for the roll. So put this behind a flag, wait for the roll,
> then toggle the flag async after the roll. (And then the flag can be removed
> again.)

Sure, will do shortly.

[Lei Zhang, 2015-12-29 19:22:50]

See patch set 3.

[Nico, 2015-12-29 19:57:25]

https://codereview.chromium.org/1554533002/diff/40001/tools/clang/plugins/Fin...
File tools/clang/plugins/FindBadConstructsAction.cpp (right):

https://codereview.chromium.org/1554533002/diff/40001/tools/clang/plugins/Fin...
tools/clang/plugins/FindBadConstructsAction.cpp:53:
options_.enforce_in_thirdparty_pdf = true;
the flag is called enforce_in_pdf (no thirdparty_); this won't build

[Lei Zhang, 2015-12-29 20:00:17]

https://codereview.chromium.org/1554533002/diff/40001/tools/clang/plugins/Fin...
File tools/clang/plugins/FindBadConstructsAction.cpp (right):

https://codereview.chromium.org/1554533002/diff/40001/tools/clang/plugins/Fin...
tools/clang/plugins/FindBadConstructsAction.cpp:53:
options_.enforce_in_thirdparty_pdf = true;
On 2015/12/29 19:57:25, Nico wrote:
> the flag is called enforce_in_pdf (no thirdparty_); this won't build

copy + pasting is hard :(

[Nico, 2015-12-29 20:05:42]

lgtm if you land my breakpad cl :-D

[Lei Zhang, 2015-12-29 20:10:14]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2015-12-29 20:10:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1554533002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1554533002/60001

[commit-bot: I haz the power, 2015-12-29 21:49:26]

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

[commit-bot: I haz the power, 2015-12-29 21:49:27]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Lei Zhang, 2015-12-29 21:57:25]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2015-12-29 21:58:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1554533002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1554533002/60001

[commit-bot: I haz the power, 2015-12-29 22:38:22]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-12-29 22:39:29]

Description was changed from

==========
Mark methods with override in pdf/

Fix some small nits along the way.
==========

to

==========
Mark methods with override in pdf/

Fix some small nits along the way.

Committed: https://crrev.com/98638620100303034cd94c7a02e8c64be0bd93b7
Cr-Commit-Position: refs/heads/master@{#367109}
==========

[commit-bot: I haz the power, 2015-12-29 22:39:30]

Patchset 4 (id:??) landed as
https://crrev.com/98638620100303034cd94c7a02e8c64be0bd93b7
Cr-Commit-Position: refs/heads/master@{#367109}

[Nico, 2016-08-22 19:24:02]

https://codereview.chromium.org/1554533002/diff/40001/tools/clang/plugins/Fin...
File tools/clang/plugins/FindBadConstructsAction.cpp (right):

https://codereview.chromium.org/1554533002/diff/40001/tools/clang/plugins/Fin...
tools/clang/plugins/FindBadConstructsAction.cpp:53:
options_.enforce_in_thirdparty_pdf = true;
You never turned on this flag -- are you still planning on doing that?

[Lei Zhang, 2016-08-22 19:27:58]

On 2016/08/22 19:24:02, Nico wrote:
>
https://codereview.chromium.org/1554533002/diff/40001/tools/clang/plugins/Fin...
> File tools/clang/plugins/FindBadConstructsAction.cpp (right):
> 
>
https://codereview.chromium.org/1554533002/diff/40001/tools/clang/plugins/Fin...
> tools/clang/plugins/FindBadConstructsAction.cpp:53:
> options_.enforce_in_thirdparty_pdf = true;
> You never turned on this flag -- are you still planning on doing that?

Thanks for reminding me. I guess I should give it a whirl.