New issue
Advanced search Search tips

Issue 667584 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Extensions code shouldn't pass source tab id in the request message

Project Member Reported by rdevlin....@chromium.org, Nov 22 2016

Issue description

Extension requests (i.e., API calls) include the "source tab id" in the message params [1]. However, since this message is routed through a render frame, we shouldn't need to include the tab id - the browser should know it.  What's more, the current source tab id is retrieved from the request sender [2], which only has it set for messaging calls [3]... which is all very wrong.

[1] https://chromium.googlesource.com/chromium/src/+/ff516ab47c3fe130da26006e0f677ee5a78a9f19/extensions/common/extension_messages.h#104
[2] https://chromium.googlesource.com/chromium/src/+/ff516ab47c3fe130da26006e0f677ee5a78a9f19/extensions/renderer/request_sender.cc#114
[3] https://chromium.googlesource.com/chromium/src/+/ff516ab47c3fe130da26006e0f677ee5a78a9f19/extensions/renderer/dispatcher.cc#1039
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ae224f1bfe3875919fa6c79130a31affb6e01f2f

commit ae224f1bfe3875919fa6c79130a31affb6e01f2f
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Wed Nov 23 02:58:08 2016

[Extensions] Remove ChromeExtensionFunctionDetails::GetOriginWebContents

Remove the ChromeExtensionFunctionDetails::GetOriginWebContents()
method. There are a few problems with this function:
- Like GetAssociatedWebContents(), it is unclear and unpredictable (see
  also  crbug.com/461394 ). It's not obvious what the returned result will
  be, and it's very likely *not* what the caller is expecting from the
  title or function comment.
- It uses ExtensionFunction::source_tab_id, which needs to be removed.

Remove the function and implement the necessary pieces directly in the
previous callers.

BUG= 667584 

Review-Url: https://codereview.chromium.org/2521363002
Cr-Commit-Position: refs/heads/master@{#434097}

[modify] https://crrev.com/ae224f1bfe3875919fa6c79130a31affb6e01f2f/chrome/browser/extensions/api/media_galleries/media_galleries_api.cc
[modify] https://crrev.com/ae224f1bfe3875919fa6c79130a31affb6e01f2f/chrome/browser/extensions/chrome_extension_function_details.cc
[modify] https://crrev.com/ae224f1bfe3875919fa6c79130a31affb6e01f2f/chrome/browser/extensions/chrome_extension_function_details.h

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 28 2016

Status: Fixed (was: Assigned)

Sign in to add a comment