Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(155)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 516123004: Support ServiceWorker created request objects. (Closed)

Created:
6 years, 3 months ago by gavinp
Modified:
6 years, 3 months ago
CC:
abarth-chromium, blink-reviews, dglazkov+blink, falken, horo+watch_chromium.org, jamesr, jsbell+serviceworker_chromium.org, kinuko+serviceworker, michaeln, nhiroki, serviceworker-reviews, tzik
Base URL:
https://chromium.googlesource.com/chromium/blink.git@minimaster
Project:
blink
Visibility:
Public.

Description

Support ServiceWorker created request objects. Request objects previously were created in the embedder and provided to Blink; however the Cache API requires that they can cross the other way. This CL adds the accessors required to do this; both to fill in a WebServiceWorkerRequest object in Blink, and to read all the fields of the WebServiceWorkerRequest in the embedder. R=jkarlin@chromium.org,jsbell@chromium.org,asanka@chromium.org BUG=399178 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181322

Patch Set 1 #

Patch Set 2 : add test #

Patch Set 3 : fix release build #

Patch Set 4 : improved test #

Total comments: 4

Patch Set 5 : remediate to jochen review #

Patch Set 6 : rebase to trunk #

Patch Set 7 : fix build #

Total comments: 1

Patch Set 8 : fix more builds #

Unified diffs Side-by-side diffs Delta from patch set Stats (+161 lines, -6 lines) Patch
M Source/modules/modules.gypi View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M Source/modules/serviceworkers/Request.h View 1 2 3 4 5 2 chunks +2 lines, -2 lines 0 comments Download
M Source/modules/serviceworkers/Request.cpp View 1 2 3 4 5 3 chunks +31 lines, -0 lines 0 comments Download
A Source/modules/serviceworkers/RequestTest.cpp View 1 2 3 4 5 6 7 1 chunk +101 lines, -0 lines 0 comments Download
M Source/platform/exported/WebServiceWorkerRequest.cpp View 1 2 3 4 3 chunks +18 lines, -0 lines 0 comments Download
M Source/platform/network/HTTPHeaderMap.h View 1 2 3 4 5 6 1 chunk +2 lines, -1 line 0 comments Download
M public/platform/WebServiceWorkerRequest.h View 1 2 3 4 2 chunks +6 lines, -3 lines 0 comments Download

Messages

Total messages: 17 (4 generated)
jsbell
lgtm
6 years, 3 months ago (2014-08-29 17:38:56 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/gavinp@chromium.org/516123004/60001
6 years, 3 months ago (2014-08-29 17:42:21 UTC) #3
gavinp
Fancy, I got an lgtm before I sent it out for review!
6 years, 3 months ago (2014-08-29 17:43:04 UTC) #4
jsbell
On 2014/08/29 17:43:04, gavinp wrote: > Fancy, I got an lgtm before I sent it ...
6 years, 3 months ago (2014-08-29 18:51:09 UTC) #5
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: blink_presubmit on tryserver.blink ...
6 years, 3 months ago (2014-08-29 18:51:49 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: blink_presubmit on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/13999)
6 years, 3 months ago (2014-08-29 18:55:40 UTC) #8
jamesr
Sorry, this looks out of my depth. Think you'll want tkent or darin.
6 years, 3 months ago (2014-08-29 19:07:27 UTC) #9
gavinp
On 2014/09/02 13:13:59, gavinp wrote: > mailto:gavinp@chromium.org changed reviewers: > + mailto:jochen@chromium.org jochen: PTAL, this ...
6 years, 3 months ago (2014-09-02 13:14:12 UTC) #11
jochen (gone - plz use gerrit)
lgtm with comment adressed https://codereview.chromium.org/516123004/diff/60001/Source/modules/serviceworkers/RequestTest.cpp File Source/modules/serviceworkers/RequestTest.cpp (right): https://codereview.chromium.org/516123004/diff/60001/Source/modules/serviceworkers/RequestTest.cpp#newcode91 Source/modules/serviceworkers/RequestTest.cpp:91: EXPECT_EQ(referrer, KURL(secondWebRequest.referrerUrl()).string()); and test it ...
6 years, 3 months ago (2014-09-03 09:12:57 UTC) #12
gavinp
TY for the review! https://codereview.chromium.org/516123004/diff/60001/Source/modules/serviceworkers/RequestTest.cpp File Source/modules/serviceworkers/RequestTest.cpp (right): https://codereview.chromium.org/516123004/diff/60001/Source/modules/serviceworkers/RequestTest.cpp#newcode91 Source/modules/serviceworkers/RequestTest.cpp:91: EXPECT_EQ(referrer, KURL(secondWebRequest.referrerUrl()).string()); On 2014/09/03 09:12:57, ...
6 years, 3 months ago (2014-09-03 11:45:59 UTC) #13
gavinp
https://codereview.chromium.org/516123004/diff/120001/Source/platform/network/HTTPHeaderMap.h File Source/platform/network/HTTPHeaderMap.h (right): https://codereview.chromium.org/516123004/diff/120001/Source/platform/network/HTTPHeaderMap.h#newcode68 Source/platform/network/HTTPHeaderMap.h:68: bool operator==(const HTTPHeaderMap& rhs) const { return m_headers == ...
6 years, 3 months ago (2014-09-03 13:14:58 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/gavinp@chromium.org/516123004/140001
6 years, 3 months ago (2014-09-03 14:50:26 UTC) #16
commit-bot: I haz the power
6 years, 3 months ago (2014-09-03 15:08:13 UTC) #17
Message was sent while issue was closed. 
Committed patchset #8 (id:140001) as 181322

Powered by Google App Engine
This is Rietveld 408576698