Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Side by Side Diff: src/GlobalJsObject.cpp

Issue 29393573: Issue 4692 - Dont' keep a strong reference from timer thread (Closed)
Patch Set: Created March 23, 2017, 5:24 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
OLDNEW
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-2016 Eyeo GmbH 3 * Copyright (C) 2006-2016 Eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
(...skipping 18 matching lines...) Expand all
29 #include "Thread.h" 29 #include "Thread.h"
30 #include "Utils.h" 30 #include "Utils.h"
31 31
32 using namespace AdblockPlus; 32 using namespace AdblockPlus;
33 33
34 namespace 34 namespace
35 { 35 {
36 class TimeoutThread : public Thread 36 class TimeoutThread : public Thread
37 { 37 {
38 public: 38 public:
39 TimeoutThread(JsValueList& arguments) 39 TimeoutThread(const JsEngine::TimerTask& timerTask)
40 : Thread(true) 40 : Thread(true)
41 , timerTask(timerTask)
Oleksandr 2017/03/24 12:15:25 Nit: I think our coding style would put a comma on
sergei 2017/03/24 12:34:39 Done.
41 { 42 {
42 if (arguments.size() < 2)
43 throw std::runtime_error("setTimeout requires at least 2 parameters");
44
45 if (!arguments[0]->IsFunction())
46 throw std::runtime_error(
47 "First argument to setTimeout must be a function");
48
49 function = arguments[0];
50 delay = arguments[1]->AsInt();
51 for (size_t i = 2; i < arguments.size(); i++)
52 functionArguments.push_back(arguments[i]);
53 } 43 }
54 44
55 void Run() 45 void Run()
56 { 46 {
57 Sleep(delay); 47 Sleep(timerTask.ii_taskInfo->delay);
58 48 if (auto jsEngine = timerTask.weakJsEngine.lock())
59 function->Call(functionArguments); 49 jsEngine->CallTimerTask(timerTask.ii_taskInfo);
60 } 50 }
61 51
62 private: 52 private:
63 JsValuePtr function; 53 JsEngine::TimerTask timerTask;
64 int delay;
65 JsValueList functionArguments;
66 }; 54 };
67 55
68 v8::Handle<v8::Value> SetTimeoutCallback(const v8::Arguments& arguments) 56 v8::Handle<v8::Value> SetTimeoutCallback(const v8::Arguments& arguments)
69 { 57 {
70 TimeoutThread* timeoutThread; 58 TimeoutThread* timeoutThread;
71 try 59 try
72 { 60 {
73 AdblockPlus::JsValueList converted = 61 auto jsEngine = AdblockPlus::JsEngine::FromArguments(arguments);
74 AdblockPlus::JsEngine::FromArguments(arguments) 62 timeoutThread = new TimeoutThread(jsEngine->CreateTimerTask(arguments));
75 ->ConvertArguments(arguments);
76 timeoutThread = new TimeoutThread(converted);
77 } 63 }
78 catch (const std::exception& e) 64 catch (const std::exception& e)
79 { 65 {
80 v8::Isolate* isolate = arguments.GetIsolate(); 66 v8::Isolate* isolate = arguments.GetIsolate();
81 return v8::ThrowException(Utils::ToV8String(isolate, e.what())); 67 return v8::ThrowException(Utils::ToV8String(isolate, e.what()));
82 } 68 }
83 timeoutThread->Start(); 69 timeoutThread->Start();
84 70
85 // We should actually return the timer ID here, which could be 71 // We should actually return the timer ID here, which could be
86 // used via clearTimeout(). But since we don't seem to need 72 // used via clearTimeout(). But since we don't seem to need
(...skipping 26 matching lines...) Expand all
113 obj->SetProperty("_fileSystem", 99 obj->SetProperty("_fileSystem",
114 FileSystemJsObject::Setup(jsEngine, jsEngine->NewObject())); 100 FileSystemJsObject::Setup(jsEngine, jsEngine->NewObject()));
115 obj->SetProperty("_webRequest", 101 obj->SetProperty("_webRequest",
116 WebRequestJsObject::Setup(jsEngine, jsEngine->NewObject())); 102 WebRequestJsObject::Setup(jsEngine, jsEngine->NewObject()));
117 obj->SetProperty("console", 103 obj->SetProperty("console",
118 ConsoleJsObject::Setup(jsEngine, jsEngine->NewObject())); 104 ConsoleJsObject::Setup(jsEngine, jsEngine->NewObject()));
119 obj->SetProperty("_appInfo", 105 obj->SetProperty("_appInfo",
120 AppInfoJsObject::Setup(jsEngine, appInfo, jsEngine->NewObject())); 106 AppInfoJsObject::Setup(jsEngine, appInfo, jsEngine->NewObject()));
121 return obj; 107 return obj;
122 } 108 }
OLDNEW

Powered by Google App Engine
This is Rietveld