Cleaning up capture_web_contents_function.

Cleaning up capture_web_contents_function.

BUG=411430
Committed: https://crrev.com/18533307aef4422946f1f242d6aced1b7aea870b
Cr-Commit-Position: refs/heads/master@{#293998}

[lfg, 2014-09-05 17:12:01]

lfg@chromium.org changed reviewers:
+ rockot@chromium.org

[Ken Rockot(use gerrit already), 2014-09-05 19:55:32]

lgtm pending base landing

https://codereview.chromium.org/545973002/diff/1/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_tab_util.cc (right):

https://codereview.chromium.org/545973002/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_tab_util.cc:281: Browser*
ExtensionTabUtil::GetBrowserFromWindowID(
Alternatively I guess CEFDetails could provide a public function() accessor and
you could axe the function argument to this method. Up to you.

https://codereview.chromium.org/545973002/diff/1/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_tab_util.h (right):

https://codereview.chromium.org/545973002/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_tab_util.h:78: // DEPRECATED: Please
consider using ChromeExtensionFunctionDetails.
nit: Maybe point out the relevant overload of the function below as well.

[Fady Samuel, 2014-09-05 19:56:45]

fsamuel@chromium.org changed reviewers:
+ fsamuel@chromium.org

[Fady Samuel, 2014-09-05 19:56:46]

Could you please add more detailed descriptions as to what the change is?

[Fady Samuel, 2014-09-05 19:56:55]

On 2014/09/05 19:56:46, Fady Samuel wrote:
> Could you please add more detailed descriptions as to what the change is?

Also include a bug number.

[lfg, 2014-09-05 20:00:19]

On 2014/09/05 19:56:55, Fady Samuel wrote:
> On 2014/09/05 19:56:46, Fady Samuel wrote:
> > Could you please add more detailed descriptions as to what the change is?
> 
> Also include a bug number.

It's just a cleanup to remove the templated inheritance and use the new
interface that Ken added. There's no bug associated, and doesn't fix anything.

[Ken Rockot(use gerrit already), 2014-09-05 20:01:39]

Use bug 411430, I filed a generic bug to deprecate Chrome*Function


On Fri, Sep 5, 2014 at 1:00 PM, <lfg@chromium.org> wrote:

> On 2014/09/05 19:56:55, Fady Samuel wrote:
>
>> On 2014/09/05 19:56:46, Fady Samuel wrote:
>> > Could you please add more detailed descriptions as to what the change
>> is?
>>
>
>  Also include a bug number.
>>
>
> It's just a cleanup to remove the templated inheritance and use the new
> interface that Ken added. There's no bug associated, and doesn't fix
> anything.
>
>
>
> https://codereview.chromium.org/545973002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Fady Samuel, 2014-09-08 14:54:52]

https://codereview.chromium.org/545973002/diff/1/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_tab_util.h (right):

https://codereview.chromium.org/545973002/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_tab_util.h:78: // DEPRECATED: Please
consider using ChromeExtensionFunctionDetails.
On 2014/09/05 19:55:31, Ken Rockot wrote:
> nit: Maybe point out the relevant overload of the function below as well.

Also, the typical style is TODO(ldap): ...

https://codereview.chromium.org/545973002/diff/1/extensions/browser/api/execu...
File extensions/browser/api/execute_code_function.cc (left):

https://codereview.chromium.org/545973002/diff/1/extensions/browser/api/execu...
extensions/browser/api/execute_code_function.cc:29: 
Undo this change.

[lfg, 2014-09-08 20:28:11]

On 2014/09/05 19:55:32, Ken Rockot wrote:
> lgtm pending base landing
> 
>
https://codereview.chromium.org/545973002/diff/1/chrome/browser/extensions/ex...
> File chrome/browser/extensions/extension_tab_util.cc (right):
> 
>
https://codereview.chromium.org/545973002/diff/1/chrome/browser/extensions/ex...
> chrome/browser/extensions/extension_tab_util.cc:281: Browser*
> ExtensionTabUtil::GetBrowserFromWindowID(
> Alternatively I guess CEFDetails could provide a public function() accessor
and
> you could axe the function argument to this method. Up to you.

Done.

>
https://codereview.chromium.org/545973002/diff/1/chrome/browser/extensions/ex...
> File chrome/browser/extensions/extension_tab_util.h (right):
> 
>
https://codereview.chromium.org/545973002/diff/1/chrome/browser/extensions/ex...
> chrome/browser/extensions/extension_tab_util.h:78: // DEPRECATED: Please
> consider using ChromeExtensionFunctionDetails.
> nit: Maybe point out the relevant overload of the function below as well.

Done.

[lfg, 2014-09-08 20:29:56]

On 2014/09/08 14:54:52, Fady Samuel wrote:
>
https://codereview.chromium.org/545973002/diff/1/chrome/browser/extensions/ex...
> File chrome/browser/extensions/extension_tab_util.h (right):
> 
>
https://codereview.chromium.org/545973002/diff/1/chrome/browser/extensions/ex...
> chrome/browser/extensions/extension_tab_util.h:78: // DEPRECATED: Please
> consider using ChromeExtensionFunctionDetails.
> On 2014/09/05 19:55:31, Ken Rockot wrote:
> > nit: Maybe point out the relevant overload of the function below as well.
> 
> Also, the typical style is TODO(ldap): ...

Not sure I understand, am I not supposed to put the deprecated? I've followed
the same pattern as:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...

>
https://codereview.chromium.org/545973002/diff/1/extensions/browser/api/execu...
> File extensions/browser/api/execute_code_function.cc (left):
> 
>
https://codereview.chromium.org/545973002/diff/1/extensions/browser/api/execu...
> extensions/browser/api/execute_code_function.cc:29: 
> Undo this change.

Done. For some reason, git cl format does that automatically.

[Fady Samuel, 2014-09-09 16:29:31]

lgtm

[lfg, 2014-09-09 16:31:30]

The CQ bit was checked by lfg@chromium.org

[commit-bot: I haz the power, 2014-09-09 16:53:33]

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

[lfg, 2014-09-09 16:59:36]

The CQ bit was checked by lfg@chromium.org

[commit-bot: I haz the power, 2014-09-09 17:13:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/lfg@chromium.org/545973002/60001

[commit-bot: I haz the power, 2014-09-09 20:21:22]

Committed patchset #4 (id:60001) as e493bf0983ab9cbc713ad39c519c3f214296d020

[commit-bot: I haz the power, 2014-09-10 03:55:15]

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